diff --git a/x/ibc-transfer/keeper/grpc_query.go b/x/ibc-transfer/keeper/grpc_query.go index abde23bd8..dcc711b96 100644 --- a/x/ibc-transfer/keeper/grpc_query.go +++ b/x/ibc-transfer/keeper/grpc_query.go @@ -67,7 +67,7 @@ func (q Keeper) DenomTraces(c context.Context, req *types.QueryDenomTracesReques } return &types.QueryDenomTracesResponse{ - DenomTraces: traces, + DenomTraces: traces.Sort(), Pagination: pageRes, }, nil } diff --git a/x/ibc-transfer/keeper/relay.go b/x/ibc-transfer/keeper/relay.go index dc48f13db..4fc9e7c15 100644 --- a/x/ibc-transfer/keeper/relay.go +++ b/x/ibc-transfer/keeper/relay.go @@ -256,7 +256,10 @@ func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, dat func (k Keeper) refundPacketToken(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData) error { // NOTE: packet data type already checked in handler.go - token := sdk.NewCoin(data.Denom, sdk.NewIntFromUint64(data.Amount)) + // parse the denomination from the full denom path + trace := types.ParseDenomTrace(data.Denom) + + token := sdk.NewCoin(trace.IBCDenom(), sdk.NewIntFromUint64(data.Amount)) // decode the sender address sender, err := sdk.AccAddressFromBech32(data.Sender) @@ -264,7 +267,7 @@ func (k Keeper) refundPacketToken(ctx sdk.Context, packet channeltypes.Packet, d return err } - if types.SenderChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), token.Denom) { + if types.SenderChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) { // unescrow tokens back to sender escrowAddress := types.GetEscrowAddress(packet.GetSourcePort(), packet.GetSourceChannel()) return k.bankKeeper.SendCoins(ctx, escrowAddress, sender, sdk.NewCoins(token)) diff --git a/x/ibc-transfer/keeper/relay_test.go b/x/ibc-transfer/keeper/relay_test.go index 0b17f6301..1a25f5d40 100644 --- a/x/ibc-transfer/keeper/relay_test.go +++ b/x/ibc-transfer/keeper/relay_test.go @@ -1,90 +1,92 @@ package keeper_test -// TODO: these tests are blocked on multiple bugs in the existing ibc-transfer -// code -// - https://github.com/cosmos/cosmos-sdk/issues/6649 -// - https://github.com/cosmos/cosmos-sdk/issues/6827 - -/* import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/ibc-transfer/types" - clientexported "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" + clienttypes "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types" channeltypes "github.com/cosmos/cosmos-sdk/x/ibc/04-channel/types" host "github.com/cosmos/cosmos-sdk/x/ibc/24-host" + "github.com/cosmos/cosmos-sdk/x/ibc/exported" ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing" ) -// test sending from chainA to chainB using both coins that orignate on -// chainA and coins that orignate on chainB +// test sending from chainA to chainB using both coin that orignate on +// chainA and coin that orignate on chainB func (suite *KeeperTestSuite) TestSendTransfer() { var ( - amount sdk.Coins + amount sdk.Coin channelA, channelB ibctesting.TestChannel err error ) testCases := []struct { - msg string - malleate func() - source bool - expPass bool + msg string + malleate func() + sendFromSource bool + expPass bool }{ {"successful transfer from source chain", func() { - _, _, _, _, channelA, channelB = suite.coordinator.Setup(suite.chainA, suite.chainB) - amount = types.GetTransferCoin(channelB, sdk.DefaultBondDenom, 100) + _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint) + channelA, channelB = suite.coordinator.CreateTransferChannels(suite.chainA, suite.chainB, connA, connB, channeltypes.UNORDERED) + amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)) }, true, true}, - {"successful transfer with coins from counterparty chain", + {"successful transfer with coin from counterparty chain", func() { - // send coins from chainA back to chainB - _, _, _, _, channelA, channelB = suite.coordinator.Setup(suite.chainA, suite.chainB) - amount = types.GetTransferCoin(channelA, sdk.DefaultBondDenom, 100) + // send coin from chainA back to chainB + _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint) + channelA, channelB = suite.coordinator.CreateTransferChannels(suite.chainA, suite.chainB, connA, connB, channeltypes.UNORDERED) + amount = types.GetTransferCoin(channelA.PortID, channelA.ID, sdk.DefaultBondDenom, 100) }, false, true}, {"source channel not found", func() { // channel references wrong ID - _, _, _, _, channelA, channelB = suite.coordinator.Setup(suite.chainA, suite.chainB) + _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint) + channelA, channelB = suite.coordinator.CreateTransferChannels(suite.chainA, suite.chainB, connA, connB, channeltypes.UNORDERED) channelA.ID = ibctesting.InvalidID - amount = types.GetTransferCoin(channelB, sdk.DefaultBondDenom, 100) + amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)) }, true, false}, {"next seq send not found", func() { - _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, clientexported.Tendermint) - channelA = connA.NextTestChannel() - channelB = connB.NextTestChannel() + _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint) + channelA = connA.NextTestChannel(ibctesting.TransferPort) + channelB = connB.NextTestChannel(ibctesting.TransferPort) // manually create channel so next seq send is never set suite.chainA.App.IBCKeeper.ChannelKeeper.SetChannel( suite.chainA.GetContext(), channelA.PortID, channelA.ID, - channeltypes.NewChannel(channeltypes.OPEN, channeltypes.ORDERED, channeltypes.NewCounterparty(channelB.PortID, channelB.ID), []string{connA.ID}, ibctesting.ChannelVersion), + channeltypes.NewChannel(channeltypes.OPEN, channeltypes.ORDERED, channeltypes.NewCounterparty(channelB.PortID, channelB.ID), []string{connA.ID}, ibctesting.DefaultChannelVersion), ) suite.chainA.CreateChannelCapability(channelA.PortID, channelA.ID) - amount = types.GetTransferCoin(channelB, sdk.DefaultBondDenom, 100) + amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)) }, true, false}, // createOutgoingPacket tests // - source chain - {"send coins failed", + {"send coin failed", func() { - _, _, _, _, channelA, channelB = suite.coordinator.Setup(suite.chainA, suite.chainB) - amount = types.GetTransferCoin(channelB, "randomdenom", 100) + _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint) + channelA, channelB = suite.coordinator.CreateTransferChannels(suite.chainA, suite.chainB, connA, connB, channeltypes.UNORDERED) + amount = sdk.NewCoin("randomdenom", sdk.NewInt(100)) }, true, false}, // - receiving chain {"send from module account failed", func() { - _, _, _, _, channelA, channelB = suite.coordinator.Setup(suite.chainA, suite.chainB) - amount = types.GetTransferCoin(channelA, "randomdenom", 100) + _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint) + channelA, channelB = suite.coordinator.CreateTransferChannels(suite.chainA, suite.chainB, connA, connB, channeltypes.UNORDERED) + amount = types.GetTransferCoin(channelA.PortID, channelA.ID, " randomdenom", 100) }, false, false}, {"channel capability not found", func() { - _, _, _, _, channelA, channelB = suite.coordinator.Setup(suite.chainA, suite.chainB) + _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint) + channelA, channelB = suite.coordinator.CreateTransferChannels(suite.chainA, suite.chainB, connA, connB, channeltypes.UNORDERED) cap := suite.chainA.GetChannelCapability(channelA.PortID, channelA.ID) // Release channel capability suite.chainA.App.ScopedTransferKeeper.ReleaseCapability(suite.chainA.GetContext(), cap) + amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)) }, true, false}, } @@ -96,29 +98,29 @@ func (suite *KeeperTestSuite) TestSendTransfer() { tc.malleate() - if !tc.source { - // send coins from chainB to chainA - coinFromBToA := types.GetTransferCoin(channelA, sdk.DefaultBondDenom, 100) - transferMsg := types.NewMsgTransfer(channelB.PortID, channelB.ID, coinFromBToA, suite.chainB.SenderAccount.GetAddress(), suite.chainA.SenderAccount.GetAddress().String(), 110, 0) - err = suite.coordinator.SendMsgs(suite.chainB, suite.chainA, channelA.ClientID, transferMsg) + if !tc.sendFromSource { + // send coin from chainB to chainA + coinFromBToA := sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)) + transferMsg := types.NewMsgTransfer(channelB.PortID, channelB.ID, coinFromBToA, suite.chainB.SenderAccount.GetAddress(), suite.chainA.SenderAccount.GetAddress().String(), clienttypes.NewHeight(0, 110), 0) + err = suite.coordinator.SendMsg(suite.chainB, suite.chainA, channelA.ClientID, transferMsg) suite.Require().NoError(err) // message committed - // receive coins on chainA from chainB - fungibleTokenPacket := types.NewFungibleTokenPacketData(coinFromBToA, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) - packet := channeltypes.NewPacket(fungibleTokenPacket.GetBytes(), 1, channelB.PortID, channelB.ID, channelA.PortID, channelA.ID, 110, 0) + // receive coin on chainA from chainB + fungibleTokenPacket := types.NewFungibleTokenPacketData(coinFromBToA.Denom, coinFromBToA.Amount.Uint64(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) + packet := channeltypes.NewPacket(fungibleTokenPacket.GetBytes(), 1, channelB.PortID, channelB.ID, channelA.PortID, channelA.ID, clienttypes.NewHeight(0, 110), 0) // get proof of packet commitment from chainB packetKey := host.KeyPacketCommitment(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) proof, proofHeight := suite.chainB.QueryProof(packetKey) recvMsg := channeltypes.NewMsgRecvPacket(packet, proof, proofHeight, suite.chainA.SenderAccount.GetAddress()) - err = suite.coordinator.SendMsgs(suite.chainA, suite.chainB, channelB.ClientID, recvMsg) + err = suite.coordinator.SendMsg(suite.chainA, suite.chainB, channelB.ClientID, recvMsg) suite.Require().NoError(err) // message committed } err = suite.chainA.App.TransferKeeper.SendTransfer( suite.chainA.GetContext(), channelA.PortID, channelA.ID, amount, - suite.chainA.SenderAccount.GetAddress(), suite.chainB.SenderAccount.GetAddress().String(), 110, 0, + suite.chainA.SenderAccount.GetAddress(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(0, 110), 0, ) if tc.expPass { @@ -130,46 +132,44 @@ func (suite *KeeperTestSuite) TestSendTransfer() { } } -// test receiving coins on chainB with coins that orignate on chainA and -// coins that orignated on chainB. Coins from source (chainA) have channelB -// as the denom prefix. The bulk of the testing occurs in the test case -// for loop since setup is intensive for all cases. The malleate function -// allows for testing invalid cases. +// test receiving coin on chainB with coin that orignate on chainA and +// coin that orignated on chainB (source). The bulk of the testing occurs +// in the test case for loop since setup is intensive for all cases. The +// malleate function allows for testing invalid cases. func (suite *KeeperTestSuite) TestOnRecvPacket() { var ( channelA, channelB ibctesting.TestChannel - coins sdk.Coins + trace types.DenomTrace + amount sdk.Int receiver string ) testCases := []struct { - msg string - malleate func() - source bool - expPass bool + msg string + malleate func() + recvIsSource bool // the receiving chain is the source of the coin originally + expPass bool }{ - {"success receive from source chain", func() {}, true, true}, - {"success receive with coins orignated on this chain", func() {}, false, true}, - {"empty amount", func() { - coins = nil + {"success receive on source chain", func() {}, true, true}, + {"success receive with coin from another chain as source", func() {}, false, true}, + {"empty coin", func() { + trace = types.DenomTrace{} + amount = sdk.ZeroInt() }, true, false}, {"invalid receiver address", func() { receiver = "gaia1scqhwpgsmr6vmztaa7suurfl52my6nd2kmrudl" }, true, false}, - {"no dest prefix on coin denom", func() { - coins = sdk.NewCoins(sdk.NewInt64Coin("bitcoin", 100)) - }, false, false}, // onRecvPacket - // - coins from source chain (chainA) - {"failure: mint zero coins", func() { - coins = types.GetTransferCoin(channelB, sdk.DefaultBondDenom, 0) - }, true, false}, - - // - coins being sent back to original chain (chainB) - {"tries to unescrow more tokens than allowed", func() { - coins = types.GetTransferCoin(channelA, sdk.DefaultBondDenom, 1000000) + // - coin from chain chainA + {"failure: mint zero coin", func() { + amount = sdk.ZeroInt() }, false, false}, + + // - coin being sent back to original chain (chainB) + {"tries to unescrow more tokens than allowed", func() { + amount = sdk.NewInt(1000000) + }, true, false}, } for _, tc := range testCases { @@ -177,41 +177,45 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { suite.Run(fmt.Sprintf("Case %s", tc.msg), func() { suite.SetupTest() // reset - _, _, _, _, channelA, channelB = suite.coordinator.Setup(suite.chainA, suite.chainB) + + clientA, clientB, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint) + channelA, channelB = suite.coordinator.CreateTransferChannels(suite.chainA, suite.chainB, connA, connB, channeltypes.UNORDERED) receiver = suite.chainB.SenderAccount.GetAddress().String() // must be explicitly changed in malleate + amount = sdk.NewInt(100) // must be explicitly changed in malleate seq := uint64(1) - if !tc.source { - // send coins from chainB to chainA, receive them, acknowledge them, and send back to chainB - coinFromBToA := types.GetTransferCoin(channelA, sdk.DefaultBondDenom, 100) - transferMsg := types.NewMsgTransfer(channelB.PortID, channelB.ID, coinFromBToA, suite.chainB.SenderAccount.GetAddress(), suite.chainA.SenderAccount.GetAddress().String(), 110, 0) - err := suite.coordinator.SendMsgs(suite.chainB, suite.chainA, channelA.ClientID, transferMsg) + if tc.recvIsSource { + // send coin from chainB to chainA, receive them, acknowledge them, and send back to chainB + coinFromBToA := sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)) + transferMsg := types.NewMsgTransfer(channelB.PortID, channelB.ID, coinFromBToA, suite.chainB.SenderAccount.GetAddress(), suite.chainA.SenderAccount.GetAddress().String(), clienttypes.NewHeight(0, 110), 0) + err := suite.coordinator.SendMsg(suite.chainB, suite.chainA, channelA.ClientID, transferMsg) suite.Require().NoError(err) // message committed // relay send packet - fungibleTokenPacket := types.NewFungibleTokenPacketData(coinFromBToA, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) - packet := channeltypes.NewPacket(fungibleTokenPacket.GetBytes(), 1, channelB.PortID, channelB.ID, channelA.PortID, channelA.ID, 110, 0) + fungibleTokenPacket := types.NewFungibleTokenPacketData(coinFromBToA.Denom, coinFromBToA.Amount.Uint64(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) + packet := channeltypes.NewPacket(fungibleTokenPacket.GetBytes(), 1, channelB.PortID, channelB.ID, channelA.PortID, channelA.ID, clienttypes.NewHeight(0, 110), 0) ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)}) err = suite.coordinator.RelayPacket(suite.chainB, suite.chainA, clientB, clientA, packet, ack.GetBytes()) suite.Require().NoError(err) // relay committed seq++ - // NOTE: coins must be explicitly changed in malleate to test invalid cases - coins = types.GetTransferCoin(channelA, sdk.DefaultBondDenom, 100) + + // NOTE: trace must be explicitly changed in malleate to test invalid cases + trace = types.ParseDenomTrace(types.GetPrefixedDenom(channelA.PortID, channelA.ID, sdk.DefaultBondDenom)) } else { - coins = types.GetTransferCoin(channelB, sdk.DefaultBondDenom, 100) + trace = types.ParseDenomTrace(sdk.DefaultBondDenom) } - // send coins from chainA to chainB - transferMsg := types.NewMsgTransfer(channelA.PortID, channelA.ID, coins, suite.chainA.SenderAccount.GetAddress(), receiver, 110, 0) - err := suite.coordinator.SendMsgs(suite.chainA, suite.chainB, channelB.ClientID, transferMsg) + // send coin from chainA to chainB + transferMsg := types.NewMsgTransfer(channelA.PortID, channelA.ID, sdk.NewCoin(trace.IBCDenom(), amount), suite.chainA.SenderAccount.GetAddress(), receiver, clienttypes.NewHeight(0, 110), 0) + err := suite.coordinator.SendMsg(suite.chainA, suite.chainB, channelB.ClientID, transferMsg) suite.Require().NoError(err) // message committed tc.malleate() - data := types.NewFungibleTokenPacketData(coins, suite.chainA.SenderAccount.GetAddress().String(), receiver) - packet := channeltypes.NewPacket(data.GetBytes(), seq, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, 100, 0) + data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.Uint64(), suite.chainA.SenderAccount.GetAddress().String(), receiver) + packet := channeltypes.NewPacket(data.GetBytes(), seq, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.NewHeight(0, 100), 0) err = suite.chainB.App.TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, data) @@ -226,38 +230,49 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { // TestOnAcknowledgementPacket tests that successful acknowledgement is a no-op // and failure acknowledment leads to refund when attempting to send from chainA -// to chainB. +// to chainB. If sender is source than the denomination being refunded has no +// trace. func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() { var ( successAck = channeltypes.NewResultAcknowledgement([]byte{byte(1)}) - failedAck = channeltypes.NewErrorAcknowledgement("failed packet transfer") + failedAck = channeltypes.NewErrorAcknowledgement("failed packet transfer") channelA, channelB ibctesting.TestChannel - coins sdk.Coins + trace types.DenomTrace + amount sdk.Int ) testCases := []struct { msg string ack channeltypes.Acknowledgement malleate func() - source bool success bool // success of ack + expPass bool }{ {"success ack causes no-op", successAck, func() { - coins = types.GetTransferCoin(channelB, sdk.DefaultBondDenom, 100) + trace = types.ParseDenomTrace(types.GetPrefixedDenom(channelB.PortID, channelB.ID, sdk.DefaultBondDenom)) }, true, true}, - {"successful refund from source chain", failedAck, + {"successful refund from source chain", failedAck, func() { + escrow := types.GetEscrowAddress(channelA.PortID, channelA.ID) + trace = types.ParseDenomTrace(sdk.DefaultBondDenom) + coin := sdk.NewCoin(sdk.DefaultBondDenom, amount) + + _, err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(coin)) + suite.Require().NoError(err) + }, false, true}, + {"unsuccessful refund from source", failedAck, + func() { + trace = types.ParseDenomTrace(sdk.DefaultBondDenom) + }, false, false}, + {"successful refund from with coin from external chain", failedAck, func() { escrow := types.GetEscrowAddress(channelA.PortID, channelA.ID) - _, err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)))) - suite.Require().NoError(err) + trace = types.ParseDenomTrace(types.GetPrefixedDenom(channelA.PortID, channelA.ID, sdk.DefaultBondDenom)) + coin := sdk.NewCoin(trace.IBCDenom(), amount) - coins = types.GetTransferCoin(channelB, sdk.DefaultBondDenom, 100) - }, true, false}, - {"successful refund from external chain", failedAck, - func() { - coins = types.GetTransferCoin(channelA, sdk.DefaultBondDenom, 100) - }, false, false}, + _, err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(coin)) + suite.Require().NoError(err) + }, false, true}, } for _, tc := range testCases { @@ -265,33 +280,30 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() { suite.Run(fmt.Sprintf("Case %s", tc.msg), func() { suite.SetupTest() // reset - _, _, _, _, channelA, channelB = suite.coordinator.Setup(suite.chainA, suite.chainB) + _, _, _, _, channelA, channelB = suite.coordinator.Setup(suite.chainA, suite.chainB, channeltypes.UNORDERED) + amount = sdk.NewInt(100) // must be explicitly changed tc.malleate() - data := types.NewFungibleTokenPacketData(coins, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String()) - packet := channeltypes.NewPacket(data.GetBytes(), 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, 100, 0) + data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.Uint64(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String()) + packet := channeltypes.NewPacket(data.GetBytes(), 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.NewHeight(0, 100), 0) - var denom string - if tc.source { - // peel off ibc denom if returning back to original chain - denom = sdk.DefaultBondDenom - } else { - denom = coins[0].Denom - } - - preCoin := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), denom) + preCoin := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), trace.IBCDenom()) err := suite.chainA.App.TransferKeeper.OnAcknowledgementPacket(suite.chainA.GetContext(), packet, data, tc.ack) - suite.Require().NoError(err) + if tc.expPass { + suite.Require().NoError(err) + postCoin := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), trace.IBCDenom()) + deltaAmount := postCoin.Amount.Sub(preCoin.Amount) - postCoin := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), denom) - deltaAmount := postCoin.Amount.Sub(preCoin.Amount) + if tc.success { + suite.Require().Equal(int64(0), deltaAmount.Int64(), "successful ack changed balance") + } else { + suite.Require().Equal(amount, deltaAmount, "failed ack did not trigger refund") + } - if tc.success { - suite.Require().Equal(int64(0), deltaAmount.Int64(), "successful ack changed balance") } else { - suite.Require().Equal(coins[0].Amount, deltaAmount, "failed ack did not trigger refund") + suite.Require().Error(err) } }) } @@ -304,39 +316,48 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() { func (suite *KeeperTestSuite) TestOnTimeoutPacket() { var ( channelA, channelB ibctesting.TestChannel - coins sdk.Coins + trace types.DenomTrace + amount sdk.Int + sender string ) testCases := []struct { msg string malleate func() - source bool expPass bool }{ - {"successful timeout from source chain", + {"successful timeout from sender as source chain", func() { escrow := types.GetEscrowAddress(channelA.PortID, channelA.ID) - _, err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)))) - suite.Require().NoError(err) + trace = types.ParseDenomTrace(sdk.DefaultBondDenom) + coin := sdk.NewCoin(trace.IBCDenom(), amount) - coins = types.GetTransferCoin(channelB, sdk.DefaultBondDenom, 100) - }, true, true}, + _, err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(coin)) + suite.Require().NoError(err) + }, true}, {"successful timeout from external chain", func() { - coins = types.GetTransferCoin(channelA, sdk.DefaultBondDenom, 100) - }, false, true}, - {"no source prefix on coin denom", + escrow := types.GetEscrowAddress(channelA.PortID, channelA.ID) + trace = types.ParseDenomTrace(types.GetPrefixedDenom(channelA.PortID, channelA.ID, sdk.DefaultBondDenom)) + coin := sdk.NewCoin(trace.IBCDenom(), amount) + + _, err := suite.chainA.App.BankKeeper.AddCoins(suite.chainA.GetContext(), escrow, sdk.NewCoins(coin)) + suite.Require().NoError(err) + }, true}, + {"no balance for coin denom", func() { - coins = sdk.NewCoins(sdk.NewCoin("bitcoin", sdk.NewInt(100))) - }, false, false}, + trace = types.ParseDenomTrace("bitcoin") + }, false}, {"unescrow failed", func() { - coins = types.GetTransferCoin(channelB, sdk.DefaultBondDenom, 100) - }, true, false}, + trace = types.ParseDenomTrace(sdk.DefaultBondDenom) + }, false}, {"mint failed", func() { - coins = types.GetTransferCoin(channelA, sdk.DefaultBondDenom, 0) - }, true, false}, + trace = types.ParseDenomTrace(types.GetPrefixedDenom(channelA.PortID, channelA.ID, sdk.DefaultBondDenom)) + amount = sdk.OneInt() + sender = "invalid address" + }, false}, } for _, tc := range testCases { @@ -344,34 +365,30 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacket() { suite.Run(fmt.Sprintf("Case %s", tc.msg), func() { suite.SetupTest() // reset - _, _, _, _, channelA, channelB = suite.coordinator.Setup(suite.chainA, suite.chainB) + + _, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint) + channelA, channelB = suite.coordinator.CreateTransferChannels(suite.chainA, suite.chainB, connA, connB, channeltypes.UNORDERED) + amount = sdk.NewInt(100) // must be explicitly changed + sender = suite.chainA.SenderAccount.GetAddress().String() tc.malleate() - data := types.NewFungibleTokenPacketData(coins, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String()) - packet := channeltypes.NewPacket(data.GetBytes(), 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, 100, 0) + data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.Uint64(), sender, suite.chainB.SenderAccount.GetAddress().String()) + packet := channeltypes.NewPacket(data.GetBytes(), 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, clienttypes.NewHeight(0, 100), 0) - var denom string - if tc.source { - denom = sdk.DefaultBondDenom - } else { - denom = coins[0].Denom - } - - preCoin := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), denom) + preCoin := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), trace.IBCDenom()) err := suite.chainA.App.TransferKeeper.OnTimeoutPacket(suite.chainA.GetContext(), packet, data) - postCoin := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), denom) + postCoin := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), trace.IBCDenom()) deltaAmount := postCoin.Amount.Sub(preCoin.Amount) if tc.expPass { suite.Require().NoError(err) - suite.Require().Equal(coins[0].Amount.Int64(), deltaAmount.Int64(), "successful timeout did not trigger refund") + suite.Require().Equal(amount.Int64(), deltaAmount.Int64(), "successful timeout did not trigger refund") } else { suite.Require().Error(err) } }) } } -*/ diff --git a/x/ibc-transfer/types/coin.go b/x/ibc-transfer/types/coin.go index e2fe7dc5a..08ae9a8d3 100644 --- a/x/ibc-transfer/types/coin.go +++ b/x/ibc-transfer/types/coin.go @@ -40,11 +40,6 @@ func GetPrefixedDenom(portID, channelID, baseDenom string) string { return fmt.Sprintf("%s/%s/%s", portID, channelID, baseDenom) } -// GetPrefixedCoin creates a copy of the given coin with the prefixed denom -func GetPrefixedCoin(portID, channelID string, coin sdk.Coin) sdk.Coin { - return sdk.NewCoin(GetPrefixedDenom(portID, channelID, coin.Denom), coin.Amount) -} - // GetTransferCoin creates a transfer coin with the port ID and channel ID // prefixed to the base denom. func GetTransferCoin(portID, channelID, baseDenom string, amount int64) sdk.Coin { diff --git a/x/ibc-transfer/types/trace.go b/x/ibc-transfer/types/trace.go index dffe00501..dabf64bf6 100644 --- a/x/ibc-transfer/types/trace.go +++ b/x/ibc-transfer/types/trace.go @@ -2,8 +2,7 @@ package types import ( "encoding/hex" - "errors" - fmt "fmt" + "fmt" "sort" "strings" @@ -62,13 +61,17 @@ func (dt DenomTrace) IBCDenom() string { // GetFullDenomPath returns the full denomination according to the ICS20 specification: // tracePath + "/" + baseDenom +// If there exists no trace then the base denomination is returned. func (dt DenomTrace) GetFullDenomPath() string { + if dt.Path == "" { + return dt.BaseDenom + } return dt.GetPrefix() + dt.BaseDenom } func validateTraceIdentifiers(identifiers []string) error { if len(identifiers) == 0 || len(identifiers)%2 != 0 { - return errors.New("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}'") + return fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: %s", identifiers) } // validate correctness of port and channel identifiers