MsgTransferResponse add sequence (backport #2377) (#2464)

* MsgTransferResponse add sequence (#2377)

## Description

Returns sequence from `sendTransfer`, and returns it with the `MsgTransferResponse`. This is not an API breaking change.

Retrieving the sequence at the time of creating the transfer is necessary in the packet forward middleware for correlation with multihop packet flows.

strangelove-ventures/packet-forward-middleware#33
strangelove-ventures/interchaintest#306

Closes #1969

---

- [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [x] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [x] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing)
Existing test coverage exercises this new method due to the re-routing of `SendTransfer` through `SendPacketTransfer`
- [x] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [x] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [x] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [x] Re-reviewed `Files changed` in the Github PR explorer
- [x] Review `Codecov Report` in the comment section below once CI passes

(cherry picked from commit 3363917)

# Conflicts:
#	CHANGELOG.md
#	modules/apps/transfer/keeper/msg_server.go
#	modules/apps/transfer/keeper/relay.go
#	modules/apps/transfer/types/tx.pb.go

* fix conflicts

* fix tests

Co-authored-by: Andrew Gouin <[email protected]>
Co-authored-by: crodriguezvega <[email protected]>

CHANGELOG.md

@@ -42,6 +42,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### State Machine Breaking
 
+* (transfer) [\#2377](https://github.com/cosmos/ibc-go/pull/2377) Adding `sequence` to `MsgTransferResponse`.
+
 ### Improvements
 
 ### Features

docs/ibc/proto-docs.md

@@ -711,6 +711,11 @@ https://github.com/cosmos/ibc/tree/master/spec/app/ics-020-fungible-token-transf
 MsgTransferResponse defines the Msg/Transfer response type.
 
 
+| Field | Type | Label | Description |
+| ----- | ---- | ----- | ----------- |
+| `sequence` | [uint64](#uint64) |  | sequence number of the transfer packet sent |
+
+
 
 
 

modules/apps/transfer/keeper/msg_server.go

@@ -18,9 +18,10 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
 		return nil, err
 	}
 
-	if err := k.SendTransfer(
+	sequence, err := k.sendTransfer(
 		ctx, msg.SourcePort, msg.SourceChannel, msg.Token, sender, msg.Receiver, msg.TimeoutHeight, msg.TimeoutTimestamp,
-	); err != nil {
+	)
+	if err != nil {
 		return nil, err
 	}
 
@@ -38,5 +39,5 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
 		),
 	})
 
-	return &types.MsgTransferResponse{}, nil
+	return &types.MsgTransferResponse{Sequence: sequence}, nil
 }

modules/apps/transfer/keeper/msg_server_test.go

@@ -61,6 +61,7 @@ func (suite *KeeperTestSuite) TestMsgTransfer() {
 		res, err := suite.chainA.GetSimApp().TransferKeeper.Transfer(sdk.WrapSDKContext(suite.chainA.GetContext()), msg)
 
 		if tc.expPass {
+			suite.Require().NotEqual(res.Sequence, uint64(0))
 			suite.Require().NoError(err)
 			suite.Require().NotNil(res)
 		} else {

modules/apps/transfer/keeper/relay.go

@@ -60,18 +60,41 @@ func (k Keeper) SendTransfer(
 	timeoutHeight clienttypes.Height,
 	timeoutTimestamp uint64,
 ) error {
+	_, err := k.sendTransfer(
+		ctx,
+		sourcePort,
+		sourceChannel,
+		token,
+		sender,
+		receiver,
+		timeoutHeight,
+		timeoutTimestamp,
+	)
+	return err
+}
 
+// sendTransfer handles transfer sending logic.
+func (k Keeper) sendTransfer(
+	ctx sdk.Context,
+	sourcePort,
+	sourceChannel string,
+	token sdk.Coin,
+	sender sdk.AccAddress,
+	receiver string,
+	timeoutHeight clienttypes.Height,
+	timeoutTimestamp uint64,
+) (uint64, error) {
 	if !k.GetSendEnabled(ctx) {
-		return types.ErrSendDisabled
+		return 0, types.ErrSendDisabled
 	}
 
 	if k.bankKeeper.BlockedAddr(sender) {
-		return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to send funds", sender)
+		return 0, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to send funds", sender)
 	}
 
 	sourceChannelEnd, found := k.channelKeeper.GetChannel(ctx, sourcePort, sourceChannel)
 	if !found {
-		return sdkerrors.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", sourcePort, sourceChannel)
+		return 0, sdkerrors.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", sourcePort, sourceChannel)
 	}
 
 	destinationPort := sourceChannelEnd.GetCounterparty().GetPortID()
@@ -80,7 +103,7 @@ func (k Keeper) SendTransfer(
 	// get the next sequence
 	sequence, found := k.channelKeeper.GetNextSequenceSend(ctx, sourcePort, sourceChannel)
 	if !found {
-		return sdkerrors.Wrapf(
+		return 0, sdkerrors.Wrapf(
 			channeltypes.ErrSequenceSendNotFound,
 			"source port: %s, source channel: %s", sourcePort, sourceChannel,
 		)
@@ -90,7 +113,7 @@ func (k Keeper) SendTransfer(
 	// See spec for this logic: https://github.com/cosmos/ibc/tree/master/spec/app/ics-020-fungible-token-transfer#packet-relay
 	channelCap, ok := k.scopedKeeper.GetCapability(ctx, host.ChannelCapabilityPath(sourcePort, sourceChannel))
 	if !ok {
-		return sdkerrors.Wrap(channeltypes.ErrChannelCapabilityNotFound, "module does not own channel capability")
+		return 0, sdkerrors.Wrap(channeltypes.ErrChannelCapabilityNotFound, "module does not own channel capability")
 	}
 
 	// NOTE: denomination and hex hash correctness checked during msg.ValidateBasic
@@ -103,7 +126,7 @@ func (k Keeper) SendTransfer(
 	if strings.HasPrefix(token.Denom, "ibc/") {
 		fullDenomPath, err = k.DenomPathFromHash(ctx, token.Denom)
 		if err != nil {
-			return err
+			return 0, err
 		}
 	}
 
@@ -126,7 +149,7 @@ func (k Keeper) SendTransfer(
 		if err := k.bankKeeper.SendCoins(
 			ctx, sender, escrowAddress, sdk.NewCoins(token),
 		); err != nil {
-			return err
+			return 0, err
 		}
 
 	} else {
@@ -136,7 +159,7 @@ func (k Keeper) SendTransfer(
 		if err := k.bankKeeper.SendCoinsFromAccountToModule(
 			ctx, sender, types.ModuleName, sdk.NewCoins(token),
 		); err != nil {
-			return err
+			return 0, err
 		}
 
 		if err := k.bankKeeper.BurnCoins(
@@ -165,7 +188,7 @@ func (k Keeper) SendTransfer(
 	)
 
 	if err := k.channelKeeper.SendPacket(ctx, channelCap, packet); err != nil {
-		return err
+		return 0, err
 	}
 
 	defer func() {
@@ -184,7 +207,7 @@ func (k Keeper) SendTransfer(
 		)
 	}()
 
-	return nil
+	return sequence, nil
 }
 
 // OnRecvPacket processes a cross chain fungible token transfer. If the

proto/ibc/applications/transfer/v1/tx.proto

@@ -41,4 +41,7 @@ message MsgTransfer {
 }
 
 // MsgTransferResponse defines the Msg/Transfer response type.
-message MsgTransferResponse {}
+message MsgTransferResponse {
+  // sequence number of the transfer packet sent
+  uint64 sequence = 1;
+}