Pass sequence in refund transfers for multi-hop refunds

go.sum

@@ -118,8 +118,6 @@ github.com/cosmos/iavl v0.19.3 h1:cESO0OwTTxQm5rmyESKW+zESheDUYI7CcZDWWDwnuxg=
 github.com/cosmos/iavl v0.19.3/go.mod h1:X9PKD3J0iFxdmgNLa7b2LYWdsGd90ToV5cAONApkEPw=
 github.com/cosmos/ibc-go/v3 v3.0.0-rc2.0.20221027123302-68845e541902 h1:k6Uoi9bIX2SOjr3JG7WjNzy/QjL6WOuEfRe0rq7ARm4=
 github.com/cosmos/ibc-go/v3 v3.0.0-rc2.0.20221027123302-68845e541902/go.mod h1:VUWLHw0C3USmTQZnTdkuXXdUdLbW8zsK3lV1Ieposog=
-github.com/cosmos/ibc-go/v3 v3.3.1 h1:i8o3iPSPN8fr7AjCPQnHEKz/VfeMrxc8mjvgAw6txWk=
-github.com/cosmos/ibc-go/v3 v3.3.1/go.mod h1:V1nliDk/5q5KWr0mup7M76oN4SY0IOU371SKbCV2wN0=
 github.com/cosmos/ledger-cosmos-go v0.11.1 h1:9JIYsGnXP613pb2vPjFeMMjBI5lEDsEaF6oYorTy6J4=
 github.com/cosmos/ledger-cosmos-go v0.11.1/go.mod h1:J8//BsAGTo3OC/vDLjMRFLW6q0WAaXvHnVc7ZmE8iUY=
 github.com/cosmos/ledger-go v0.9.2 h1:Nnao/dLwaVTk1Q5U9THldpUMMXU94BOTWPddSmVB6pI=

proto/router/v1/genesis.proto

@@ -7,7 +7,6 @@ option go_package = "github.com/strangelove-ventures/packet-forward-middleware/v
 import "gogoproto/gogo.proto";
 
 // GenesisState defines the router genesis state
-
 message GenesisState {
   Params params = 1 [ (gogoproto.nullable) = false ];
 
@@ -33,6 +32,7 @@ message InFlightPacket {
   string original_sender_address = 1;
   string refund_channel_id = 2;
   string refund_port_id = 3;
-  int32 retries_remaining = 4;
-  uint64 timeout = 5;
+  uint64 refund_sequence = 4;
+  int32 retries_remaining = 5;
+  uint64 timeout = 6;
 }

router/keeper/keeper.go

@@ -1,6 +1,7 @@
 package keeper
 
 import (
+	"encoding/json"
 	"fmt"
 	"time"
 
@@ -37,13 +38,13 @@ type PacketMetadata struct {
 }
 
 type ForwardMetadata struct {
-	SourceKey string
-	Receiver  string        `json:"receiver"`
-	Port      string        `json:"port"`
-	Channel   string        `json:"channel"`
-	Timeout   time.Duration `json:"timeout"`
-	Retries   *uint8        `json:"retries"`
-	Next      string        `json:"next"`
+	Receiver       string        `json:"receiver"`
+	Port           string        `json:"port"`
+	Channel        string        `json:"channel"`
+	Timeout        time.Duration `json:"timeout"`
+	Retries        *uint8        `json:"retries,omitempty"`
+	Next           *string       `json:"next,omitempty"`
+	RefundSequence *uint64       `json:"refund_sequence,omitempty"`
 }
 
 func (m *ForwardMetadata) Validate() error {
@@ -143,7 +144,9 @@ func (k Keeper) ForwardTransferPacket(
 	)
 
 	// set memo for next transfer with next from this transfer.
-	msgTransfer.Memo = metadata.Next
+	if metadata.Next != nil {
+		msgTransfer.Memo = *metadata.Next
+	}
 
 	// send tokens to destination
 	res, err := k.transferKeeper.Transfer(
@@ -172,6 +175,7 @@ func (k Keeper) ForwardTransferPacket(
 			OriginalSenderAddress: srcPacketSender,
 			RefundChannelId:       srcPacket.DestinationChannel,
 			RefundPortId:          srcPacket.DestinationPort,
+			RefundSequence:        srcPacket.Sequence,
 			RetriesRemaining:      int32(maxRetries),
 			Timeout:               uint64(timeout.Nanoseconds()),
 		}
@@ -280,11 +284,11 @@ func (k Keeper) RemoveInFlightPacket(ctx sdk.Context, packet channeltypes.Packet
 	store.Delete(key)
 }
 
-func (k Keeper) RefundForwardedPacket(ctx sdk.Context, packet channeltypes.Packet, timeout time.Duration) {
+func (k Keeper) RefundForwardedPacket(ctx sdk.Context, channel string, port string, refundSequence uint64, packetData transfertypes.FungibleTokenPacketData, timeout time.Duration) {
 	store := ctx.KVStore(k.storeKey)
-	key := types.RefundPacketKey(packet.SourceChannel, packet.SourcePort, packet.Sequence)
+	key := types.RefundPacketKey(channel, port, refundSequence)
 	if !store.Has(key) {
-		// not a forwarded packet, ignore.
+		// this is either not a forwarded packet, or it is the final destination for the refund.
 		return
 	}
 
@@ -296,57 +300,69 @@ func (k Keeper) RefundForwardedPacket(ctx sdk.Context, packet channeltypes.Packe
 	var inFlightPacket types.InFlightPacket
 	k.cdc.MustUnmarshal(bz, &inFlightPacket)
 
-	// Parse packet data
-	var data transfertypes.FungibleTokenPacketData
-	if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
-		k.Logger(ctx).Error("packetForwardMiddleware error unmarshalling packet data for refund",
+	amount, ok := sdk.NewIntFromString(packetData.Amount)
+	if !ok {
+		k.Logger(ctx).Error("packetForwardMiddleware error parsing amount from string for multi-hop refund",
 			"key", string(key),
-			"sequence", packet.Sequence,
-			"channel-id", packet.SourceChannel,
-			"port-id", packet.SourcePort,
+			"sequence", refundSequence,
+			"channel-id", channel,
+			"port-id", port,
 			"original-sender-address", inFlightPacket.OriginalSenderAddress,
 			"refund-channel-id", inFlightPacket.RefundChannelId,
 			"refund-port-id", inFlightPacket.RefundPortId,
+			"refund-sequence", inFlightPacket.RefundSequence,
+			"amount", packetData.Amount,
 		)
 		return
 	}
 
-	amount, ok := sdk.NewIntFromString(data.Amount)
-	if !ok {
-		k.Logger(ctx).Error("packetForwardMiddleware error parsing amount from string for refund",
+	msgTransfer := transfertypes.NewMsgTransfer(
+		inFlightPacket.RefundPortId,
+		inFlightPacket.RefundChannelId,
+		sdk.NewCoin(transfertypes.ParseDenomTrace(packetData.Denom).IBCDenom(), amount),
+		packetData.Sender,
+		inFlightPacket.OriginalSenderAddress,
+		DefaultTransferPacketTimeoutHeight,
+		uint64(timeout.Nanoseconds())+uint64(ctx.BlockTime().UnixNano()),
+	)
+
+	packetMemo := &PacketMetadata{
+		Forward: &ForwardMetadata{
+			RefundSequence: &inFlightPacket.RefundSequence,
+		},
+	}
+
+	memo, err := json.Marshal(packetMemo)
+	if err != nil {
+		k.Logger(ctx).Error("packetForwardMiddleware error marshaling json for multi-hop refund sequence",
 			"key", string(key),
-			"sequence", packet.Sequence,
-			"channel-id", packet.SourceChannel,
-			"port-id", packet.SourcePort,
+			"sequence", refundSequence,
+			"channel-id", channel,
+			"port-id", port,
 			"original-sender-address", inFlightPacket.OriginalSenderAddress,
 			"refund-channel-id", inFlightPacket.RefundChannelId,
 			"refund-port-id", inFlightPacket.RefundPortId,
-			"amount", data.Amount,
+			"refund-sequence", inFlightPacket.RefundSequence,
+			"amount", packetData.Amount,
 		)
 		return
 	}
 
+	msgTransfer.Memo = string(memo)
+
 	if _, err := k.transferKeeper.Transfer(
 		sdk.WrapSDKContext(ctx),
-		transfertypes.NewMsgTransfer(
-			inFlightPacket.RefundPortId,
-			inFlightPacket.RefundChannelId,
-			sdk.NewCoin(transfertypes.ParseDenomTrace(data.Denom).IBCDenom(), amount),
-			data.Sender,
-			inFlightPacket.OriginalSenderAddress,
-			DefaultTransferPacketTimeoutHeight,
-			uint64(timeout.Nanoseconds())+uint64(ctx.BlockTime().UnixNano()),
-		),
+		msgTransfer,
 	); err != nil {
-		k.Logger(ctx).Error("packetForwardMiddleware error sending packet transfer for refund",
+		k.Logger(ctx).Error("packetForwardMiddleware error sending packet transfer for multi-hop refund",
 			"key", string(key),
-			"sequence", packet.Sequence,
-			"channel-id", packet.SourceChannel,
-			"port-id", packet.SourcePort,
+			"sequence", refundSequence,
+			"channel-id", channel,
+			"port-id", port,
 			"original-sender-address", inFlightPacket.OriginalSenderAddress,
 			"refund-channel-id", inFlightPacket.RefundChannelId,
 			"refund-port-id", inFlightPacket.RefundPortId,
-			"amount", data.Amount,
+			"amount", packetData.Amount,
 			"error", err,
 		)
 	}

router/module.go

@@ -240,6 +240,12 @@ func (am AppModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, re
 
 	metadata := m.Forward
 
+	if m.Forward.RefundSequence != nil {
+		// pass along refund to previous hop in multi-hop, note that destination channel is used here since it is the OnRecvPacket.
+		am.keeper.RefundForwardedPacket(ctx, packet.DestinationChannel, packet.DestinationPort, *m.Forward.RefundSequence, data, am.refundTimeout)
+		return am.app.OnRecvPacket(ctx, packet, relayer)
+	}
+
 	if err := metadata.Validate(); err != nil {
 		return channeltypes.NewErrorAcknowledgement(err.Error())
 	}
@@ -290,7 +296,7 @@ func (am AppModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, re
 
 		err = am.keeper.ForwardTransferPacket(ctx, nil, packet, data.Sender, data.Receiver, metadata, token, retries, timeout, []metrics.Label{})
 		if err != nil {
-			am.keeper.RefundForwardedPacket(ctx, packet, am.refundTimeout)
+			am.keeper.RefundForwardedPacket(ctx, packet.SourceChannel, packet.SourcePort, packet.Sequence, data, am.refundTimeout)
 			ack = channeltypes.NewErrorAcknowledgement(err.Error())
 		}
 	}
@@ -310,7 +316,19 @@ func (am AppModule) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes
 
 	if !ack.Success() {
 		// If acknowledgement indicates error, no retries should be attempted. Refund will be initiated now.
-		am.keeper.RefundForwardedPacket(ctx, packet, am.refundTimeout)
+
+		var data transfertypes.FungibleTokenPacketData
+		if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
+			am.keeper.Logger(ctx).Error("packetForwardMiddleware error parsing packet data from ack for refund",
+				"sequence", packet.Sequence,
+				"channel-id", packet.SourceChannel,
+				"port-id", packet.SourcePort,
+				"error", err,
+			)
+			return nil
+		}
+
+		am.keeper.RefundForwardedPacket(ctx, packet.SourceChannel, packet.SourcePort, packet.Sequence, data, am.refundTimeout)
 		return nil
 	}
 
@@ -326,7 +344,18 @@ func (am AppModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet,
 	}
 
 	if err := am.keeper.HandleTimeout(ctx, packet); err != nil {
-		am.keeper.RefundForwardedPacket(ctx, packet, am.refundTimeout)
+		var data transfertypes.FungibleTokenPacketData
+		if err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
+			am.keeper.Logger(ctx).Error("packetForwardMiddleware error parsing packet data from timeout for refund",
+				"sequence", packet.Sequence,
+				"channel-id", packet.SourceChannel,
+				"port-id", packet.SourcePort,
+				"error", err,
+			)
+			return nil
+		}
+
+		am.keeper.RefundForwardedPacket(ctx, packet.SourceChannel, packet.SourcePort, packet.Sequence, data, am.refundTimeout)
 	}
 
 	return nil

router/module_test.go

@@ -304,14 +304,17 @@ func TestOnRecvPacket_ForwardMultihop(t *testing.T) {
 			Channel:  channel2,
 		},
 	}
-	next, err := json.Marshal(nextMetadata)
+	nextBz, err := json.Marshal(nextMetadata)
 	require.NoError(t, err)
+
+	next := string(nextBz)
+
 	packetOrig := transferPacket(t, hostAddr, &keeper.PacketMetadata{
 		Forward: &keeper.ForwardMetadata{
 			Receiver: hostAddr2,
 			Port:     port,
 			Channel:  channel,
-			Next:     string(next),
+			Next:     &next,
 		},
 	})
 	packet2 := transferPacket(t, hostAddr2, nextMetadata)