Skip to content

Commit

Permalink
fix ibc-transfer receiving denom bug (#8119)
Browse files Browse the repository at this point in the history
* fix receiving denom bug

* Update x/ibc/applications/transfer/keeper/relay.go

* add testing sending 2 hops and back one

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
  • Loading branch information
colin-axner and alexanderbez committed Dec 9, 2020
1 parent 58491a5 commit af40135
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 16 deletions.
60 changes: 45 additions & 15 deletions x/ibc/applications/transfer/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,23 @@ type TransferTestSuite struct {
// testing chains used for convenience and readability
chainA *ibctesting.TestChain
chainB *ibctesting.TestChain
chainC *ibctesting.TestChain
}

func (suite *TransferTestSuite) SetupTest() {
suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2)
suite.coordinator = ibctesting.NewCoordinator(suite.T(), 3)
suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(0))
suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(1))
suite.chainC = suite.coordinator.GetChain(ibctesting.GetChainID(2))
}

// constructs a send from chainA to chainB on the established channel/connection
// and sends the same coin back from chainB to chainA.
func (suite *TransferTestSuite) TestHandleMsgTransfer() {
// setup between chainA and 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)
originalBalance := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom)
// originalBalance := suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom)
timeoutHeight := clienttypes.NewHeight(0, 110)

coinToSendToB := sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
Expand All @@ -56,35 +59,62 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() {
voucherDenomTrace := types.ParseDenomTrace(types.GetPrefixedDenom(packet.GetDestPort(), packet.GetDestChannel(), sdk.DefaultBondDenom))
balance := suite.chainB.App.BankKeeper.GetBalance(suite.chainB.GetContext(), suite.chainB.SenderAccount.GetAddress(), voucherDenomTrace.IBCDenom())

coinToSendBackToA := types.GetTransferCoin(channelB.PortID, channelB.ID, sdk.DefaultBondDenom, 100)
suite.Require().Equal(coinToSendBackToA, balance)
coinSentFromAToB := types.GetTransferCoin(channelB.PortID, channelB.ID, sdk.DefaultBondDenom, 100)
suite.Require().Equal(coinSentFromAToB, balance)

// send from chainB back to chainA
msg = types.NewMsgTransfer(channelB.PortID, channelB.ID, coinToSendBackToA, suite.chainB.SenderAccount.GetAddress(), suite.chainA.SenderAccount.GetAddress().String(), timeoutHeight, 0)
// setup between chainB to chainC
clientOnBForC, clientOnCForB, connOnBForC, connOnCForB := suite.coordinator.SetupClientConnections(suite.chainB, suite.chainC, exported.Tendermint)
channelOnBForC, channelOnCForB := suite.coordinator.CreateTransferChannels(suite.chainB, suite.chainC, connOnBForC, connOnCForB, channeltypes.UNORDERED)

err = suite.coordinator.SendMsg(suite.chainB, suite.chainA, clientA, msg)
// send from chainB to chainC
msg = types.NewMsgTransfer(channelOnBForC.PortID, channelOnBForC.ID, coinSentFromAToB, suite.chainB.SenderAccount.GetAddress(), suite.chainC.SenderAccount.GetAddress().String(), timeoutHeight, 0)

err = suite.coordinator.SendMsg(suite.chainB, suite.chainC, clientOnCForB, msg)
suite.Require().NoError(err) // message committed

// relay send
// NOTE: fungible token is prefixed with the full trace in order to verify the packet commitment
fullDenomPath := types.GetPrefixedDenom(channelOnCForB.PortID, channelOnCForB.ID, voucherDenomTrace.GetFullDenomPath())
fungibleTokenPacket = types.NewFungibleTokenPacketData(voucherDenomTrace.GetFullDenomPath(), coinSentFromAToB.Amount.Uint64(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainC.SenderAccount.GetAddress().String())
packet = channeltypes.NewPacket(fungibleTokenPacket.GetBytes(), 1, channelOnBForC.PortID, channelOnBForC.ID, channelOnCForB.PortID, channelOnCForB.ID, timeoutHeight, 0)
err = suite.coordinator.RelayPacket(suite.chainB, suite.chainC, clientOnBForC, clientOnCForB, packet, ack.GetBytes())
suite.Require().NoError(err) // relay committed

coinSentFromBToC := sdk.NewInt64Coin(types.ParseDenomTrace(fullDenomPath).IBCDenom(), 100)
balance = suite.chainC.App.BankKeeper.GetBalance(suite.chainC.GetContext(), suite.chainC.SenderAccount.GetAddress(), coinSentFromBToC.Denom)

// check that the balance is updated on chainC
suite.Require().Equal(coinSentFromBToC, balance)

// check that balance on chain B is empty
balance = suite.chainB.App.BankKeeper.GetBalance(suite.chainB.GetContext(), suite.chainB.SenderAccount.GetAddress(), coinSentFromBToC.Denom)
suite.Require().Zero(balance.Amount.Int64())

// send from chainC back to chainB
msg = types.NewMsgTransfer(channelOnCForB.PortID, channelOnCForB.ID, coinSentFromBToC, suite.chainC.SenderAccount.GetAddress(), suite.chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0)

err = suite.coordinator.SendMsg(suite.chainC, suite.chainB, clientOnBForC, msg)
suite.Require().NoError(err) // message committed

// relay send
// NOTE: fungible token is prefixed with the full trace in order to verify the packet commitment
voucherDenom := voucherDenomTrace.GetPrefix() + voucherDenomTrace.BaseDenom
fungibleTokenPacket = types.NewFungibleTokenPacketData(voucherDenom, coinToSendBackToA.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, timeoutHeight, 0)
err = suite.coordinator.RelayPacket(suite.chainB, suite.chainA, clientB, clientA, packet, ack.GetBytes())
fungibleTokenPacket = types.NewFungibleTokenPacketData(fullDenomPath, coinSentFromBToC.Amount.Uint64(), suite.chainC.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String())
packet = channeltypes.NewPacket(fungibleTokenPacket.GetBytes(), 1, channelOnCForB.PortID, channelOnCForB.ID, channelOnBForC.PortID, channelOnBForC.ID, timeoutHeight, 0)
err = suite.coordinator.RelayPacket(suite.chainC, suite.chainB, clientOnCForB, clientOnBForC, packet, ack.GetBytes())
suite.Require().NoError(err) // relay committed

balance = suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), sdk.DefaultBondDenom)
balance = suite.chainB.App.BankKeeper.GetBalance(suite.chainB.GetContext(), suite.chainB.SenderAccount.GetAddress(), coinSentFromAToB.Denom)

// check that the balance on chainA returned back to the original state
suite.Require().Equal(originalBalance, balance)
suite.Require().Equal(coinSentFromAToB, balance)

// check that module account escrow address is empty
escrowAddress := types.GetEscrowAddress(packet.GetDestPort(), packet.GetDestChannel())
balance = suite.chainA.App.BankKeeper.GetBalance(suite.chainA.GetContext(), escrowAddress, sdk.DefaultBondDenom)
balance = suite.chainB.App.BankKeeper.GetBalance(suite.chainB.GetContext(), escrowAddress, sdk.DefaultBondDenom)
suite.Require().Equal(sdk.NewCoin(sdk.DefaultBondDenom, sdk.ZeroInt()), balance)

// check that balance on chain B is empty
balance = suite.chainB.App.BankKeeper.GetBalance(suite.chainB.GetContext(), suite.chainB.SenderAccount.GetAddress(), voucherDenomTrace.IBCDenom())
balance = suite.chainC.App.BankKeeper.GetBalance(suite.chainC.GetContext(), suite.chainC.SenderAccount.GetAddress(), voucherDenomTrace.IBCDenom())
suite.Require().Zero(balance.Amount.Int64())
}

Expand Down
12 changes: 11 additions & 1 deletion x/ibc/applications/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,17 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
// remove prefix added by sender chain
voucherPrefix := types.GetDenomPrefix(packet.GetSourcePort(), packet.GetSourceChannel())
unprefixedDenom := data.Denom[len(voucherPrefix):]
token := sdk.NewCoin(unprefixedDenom, sdk.NewIntFromUint64(data.Amount))

// coin denomination used in sending from the escrow address
denom := unprefixedDenom

// The denomination used to send the coins is either the native denom or the hash of the path
// if the denomination is not native.
denomTrace := types.ParseDenomTrace(unprefixedDenom)
if denomTrace.Path != "" {
denom = denomTrace.IBCDenom()
}
token := sdk.NewCoin(denom, sdk.NewIntFromUint64(data.Amount))

// unescrow tokens
escrowAddress := types.GetEscrowAddress(packet.GetDestPort(), packet.GetDestChannel())
Expand Down

0 comments on commit af40135

Please sign in to comment.