fix: mint and transfer funds back to escrow account on timeout or ack…

… error (#172)

* chore: fix deps

* chore: fix func sig

.github/workflows/golangci-lint.yaml

@@ -25,6 +25,6 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.52
+          version: v1.55.2
           working-directory: ${{ matrix.workdir }}
           args: --timeout=5m

middleware/packet-forward-middleware/e2e/forward_timeout_test.go

@@ -201,7 +201,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)
@@ -249,4 +249,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))
 }

middleware/packet-forward-middleware/packetforward/keeper/keeper.go

@@ -140,48 +140,57 @@ func (k *Keeper) WriteAcknowledgementForForwardedPacket(
 			}
 		}
 
+		amount, ok := sdk.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 := sdk.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
 					// to burn.
 					panic(fmt.Sprintf("cannot burn coins after a successful send from escrow account to module account: %v", err))
 				}
 			}
+		} 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)
+			}
 		}
 	}
 

middleware/packet-forward-middleware/packetforward/types/expected_keepers.go

@@ -32,6 +32,8 @@ type DistributionKeeper interface {
 // BankKeeper defines the expected bank keeper
 type BankKeeper interface {
 	SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error
+	SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
 	SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
+	MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error
 	BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error
 }