diff --git a/middleware/packet-forward-middleware/e2e/forward_timeout_test.go b/middleware/packet-forward-middleware/e2e/forward_timeout_test.go index 89df868c..6fb3d940 100644 --- a/middleware/packet-forward-middleware/e2e/forward_timeout_test.go +++ b/middleware/packet-forward-middleware/e2e/forward_timeout_test.go @@ -217,7 +217,7 @@ func TestTimeoutOnForward(t *testing.T) { time.Sleep(time.Second * 11) // Restart the relayer - err = r.StartRelayer(ctx, eRep, pathAB, pathBC) + err = r.StartRelayer(ctx, eRep, pathAB, pathBC, pathCD) require.NoError(t, err) chainAHeight, err := chainA.Height(ctx) @@ -266,4 +266,149 @@ func TestTimeoutOnForward(t *testing.T) { require.True(t, firstHopEscrowBalance.Equal(zeroBal)) require.True(t, secondHopEscrowBalance.Equal(zeroBal)) require.True(t, thirdHopEscrowBalance.Equal(zeroBal)) + + // Send IBC transfer from ChainA -> ChainB -> ChainC -> ChainD that will succeed + secondHopMetadata = &PacketMetadata{ + Forward: &ForwardMetadata{ + Receiver: userD.FormattedAddress(), + Channel: cdChan.ChannelID, + Port: cdChan.PortID, + }, + } + nextBz, err = json.Marshal(secondHopMetadata) + require.NoError(t, err) + next = string(nextBz) + + firstHopMetadata = &PacketMetadata{ + Forward: &ForwardMetadata{ + Receiver: userC.FormattedAddress(), + Channel: bcChan.ChannelID, + Port: bcChan.PortID, + Next: &next, + }, + } + + memo, err = json.Marshal(firstHopMetadata) + require.NoError(t, err) + + opts = ibc.TransferOptions{ + Memo: string(memo), + } + + chainAHeight, err = chainA.Height(ctx) + require.NoError(t, err) + + transferTx, err = chainA.SendIBCTransfer(ctx, abChan.ChannelID, userA.KeyName(), transfer, opts) + require.NoError(t, err) + + _, err = testutil.PollForAck(ctx, chainA, chainAHeight, chainAHeight+30, transferTx.Packet) + require.NoError(t, err) + + err = testutil.WaitForBlocks(ctx, 5, chainA) + require.NoError(t, err) + + // Assert balances are updated to reflect tokens now being on ChainD + chainABalance, err = chainA.GetBalance(ctx, userA.FormattedAddress(), chainA.Config().Denom) + require.NoError(t, err) + + chainBBalance, err = chainB.GetBalance(ctx, userB.FormattedAddress(), firstHopIBCDenom) + require.NoError(t, err) + + chainCBalance, err = chainC.GetBalance(ctx, userC.FormattedAddress(), secondHopIBCDenom) + require.NoError(t, err) + + chainDBalance, err = chainD.GetBalance(ctx, userD.FormattedAddress(), thirdHopIBCDenom) + require.NoError(t, err) + + require.True(t, chainABalance.Equal(initBal.Sub(transferAmount))) + require.True(t, chainBBalance.Equal(zeroBal)) + require.True(t, chainCBalance.Equal(zeroBal)) + require.True(t, chainDBalance.Equal(transferAmount)) + + firstHopEscrowBalance, err = chainA.GetBalance(ctx, firstHopEscrowAccount, chainA.Config().Denom) + require.NoError(t, err) + + secondHopEscrowBalance, err = chainB.GetBalance(ctx, secondHopEscrowAccount, firstHopIBCDenom) + require.NoError(t, err) + + thirdHopEscrowBalance, err = chainC.GetBalance(ctx, thirdHopEscrowAccount, secondHopIBCDenom) + require.NoError(t, err) + + require.True(t, firstHopEscrowBalance.Equal(transferAmount)) + require.True(t, secondHopEscrowBalance.Equal(transferAmount)) + require.True(t, thirdHopEscrowBalance.Equal(transferAmount)) + + // Compose IBC tx that will attempt to go from ChainD -> ChainC -> ChainB -> ChainA but timeout between ChainB->ChainA + transfer = ibc.WalletAmount{ + Address: userC.FormattedAddress(), + Denom: thirdHopDenom, + Amount: transferAmount, + } + + secondHopMetadata = &PacketMetadata{ + Forward: &ForwardMetadata{ + Receiver: userA.FormattedAddress(), + Channel: baChan.ChannelID, + Port: baChan.PortID, + Timeout: 1 * time.Second, + }, + } + nextBz, err = json.Marshal(secondHopMetadata) + require.NoError(t, err) + next = string(nextBz) + + firstHopMetadata = &PacketMetadata{ + Forward: &ForwardMetadata{ + Receiver: userB.FormattedAddress(), + Channel: cbChan.ChannelID, + Port: cbChan.PortID, + Next: &next, + }, + } + + memo, err = json.Marshal(firstHopMetadata) + require.NoError(t, err) + + chainDHeight, err := chainD.Height(ctx) + require.NoError(t, err) + + transferTx, err = chainD.SendIBCTransfer(ctx, dcChan.ChannelID, userD.KeyName(), transfer, ibc.TransferOptions{Memo: string(memo)}) + require.NoError(t, err) + + _, err = testutil.PollForAck(ctx, chainD, chainDHeight, chainDHeight+25, transferTx.Packet) + require.NoError(t, err) + + err = testutil.WaitForBlocks(ctx, 5, chainD) + require.NoError(t, err) + + // Assert balances to ensure timeout happened and user funds are still present on ChainD + chainABalance, err = chainA.GetBalance(ctx, userA.FormattedAddress(), chainA.Config().Denom) + require.NoError(t, err) + + chainBBalance, err = chainB.GetBalance(ctx, userB.FormattedAddress(), firstHopIBCDenom) + require.NoError(t, err) + + chainCBalance, err = chainC.GetBalance(ctx, userC.FormattedAddress(), secondHopIBCDenom) + require.NoError(t, err) + + chainDBalance, err = chainD.GetBalance(ctx, userD.FormattedAddress(), thirdHopIBCDenom) + require.NoError(t, err) + + require.True(t, chainABalance.Equal(initBal.Sub(transferAmount))) + require.True(t, chainBBalance.Equal(zeroBal)) + require.True(t, chainCBalance.Equal(zeroBal)) + require.True(t, chainDBalance.Equal(transferAmount)) + + firstHopEscrowBalance, err = chainA.GetBalance(ctx, firstHopEscrowAccount, chainA.Config().Denom) + require.NoError(t, err) + + secondHopEscrowBalance, err = chainB.GetBalance(ctx, secondHopEscrowAccount, firstHopIBCDenom) + require.NoError(t, err) + + thirdHopEscrowBalance, err = chainC.GetBalance(ctx, thirdHopEscrowAccount, secondHopIBCDenom) + require.NoError(t, err) + + require.True(t, firstHopEscrowBalance.Equal(transferAmount)) + require.True(t, secondHopEscrowBalance.Equal(transferAmount)) + require.True(t, thirdHopEscrowBalance.Equal(transferAmount)) } diff --git a/middleware/packet-forward-middleware/packetforward/keeper/keeper.go b/middleware/packet-forward-middleware/packetforward/keeper/keeper.go index 3dad3e44..19763616 100644 --- a/middleware/packet-forward-middleware/packetforward/keeper/keeper.go +++ b/middleware/packet-forward-middleware/packetforward/keeper/keeper.go @@ -224,41 +224,40 @@ func (k *Keeper) WriteAcknowledgementForForwardedPacket( } } + amount, ok := sdkmath.NewIntFromString(data.Amount) + if !ok { + return fmt.Errorf("failed to parse amount from packet data for forward refund: %s", data.Amount) + } + + denomTrace := transfertypes.ParseDenomTrace(fullDenomPath) + token := sdk.NewCoin(denomTrace.IBCDenom(), amount) + + escrowAddress := transfertypes.GetEscrowAddress(packet.SourcePort, packet.SourceChannel) + refundEscrowAddress := transfertypes.GetEscrowAddress(inFlightPacket.RefundPortId, inFlightPacket.RefundChannelId) + + newToken := sdk.NewCoins(token) + if transfertypes.SenderChainIsSource(packet.SourcePort, packet.SourceChannel, fullDenomPath) { // funds were moved to escrow account for transfer, so they need to either: // - move to the other escrow account, in the case of native denom // - burn - - amount, ok := sdkmath.NewIntFromString(data.Amount) - if !ok { - return fmt.Errorf("failed to parse amount from packet data for forward refund: %s", data.Amount) - } - denomTrace := transfertypes.ParseDenomTrace(fullDenomPath) - token := sdk.NewCoin(denomTrace.IBCDenom(), amount) - - escrowAddress := transfertypes.GetEscrowAddress(packet.SourcePort, packet.SourceChannel) - if transfertypes.SenderChainIsSource(inFlightPacket.RefundPortId, inFlightPacket.RefundChannelId, fullDenomPath) { // transfer funds from escrow account for forwarded packet to escrow account going back for refund. - - refundEscrowAddress := transfertypes.GetEscrowAddress(inFlightPacket.RefundPortId, inFlightPacket.RefundChannelId) - if err := k.bankKeeper.SendCoins( - ctx, escrowAddress, refundEscrowAddress, sdk.NewCoins(token), + ctx, escrowAddress, refundEscrowAddress, newToken, ); err != nil { return fmt.Errorf("failed to send coins from escrow account to refund escrow account: %w", err) } } else { // transfer the coins from the escrow account to the module account and burn them. - if err := k.bankKeeper.SendCoinsFromAccountToModule( - ctx, escrowAddress, transfertypes.ModuleName, sdk.NewCoins(token), + ctx, escrowAddress, transfertypes.ModuleName, newToken, ); err != nil { return fmt.Errorf("failed to send coins from escrow to module account for burn: %w", err) } if err := k.bankKeeper.BurnCoins( - ctx, transfertypes.ModuleName, sdk.NewCoins(token), + ctx, transfertypes.ModuleName, newToken, ); err != nil { // NOTE: should not happen as the module account was // retrieved on the step above and it has enough balace @@ -270,6 +269,20 @@ func (k *Keeper) WriteAcknowledgementForForwardedPacket( // We move funds from the escrowAddress in both cases, // update the total escrow amount for the denom. k.unescrowToken(ctx, token) + } else { + // Funds in the escrow account were burned, + // so on a timeout or acknowledgement error we need to mint the funds back to the escrow account. + if err := k.bankKeeper.MintCoins(ctx, transfertypes.ModuleName, newToken); err != nil { + return fmt.Errorf("cannot mint coins to the %s module account: %v", transfertypes.ModuleName, err) + } + + if err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, transfertypes.ModuleName, refundEscrowAddress, newToken); err != nil { + return fmt.Errorf("cannot send coins from the %s module to the escrow account %s: %v", transfertypes.ModuleName, refundEscrowAddress, err) + } + + currentTotalEscrow := k.transferKeeper.GetTotalEscrowForDenom(ctx, token.GetDenom()) + newTotalEscrow := currentTotalEscrow.Add(token) + k.transferKeeper.SetTotalEscrowForDenom(ctx, newTotalEscrow) } }