Skip to content

Commit

Permalink
MsgTransferResponse add sequence (#2377)
Browse files Browse the repository at this point in the history
## 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:
#	modules/apps/transfer/keeper/msg_server.go
#	modules/apps/transfer/keeper/relay.go
#	modules/apps/transfer/types/tx.pb.go
  • Loading branch information
agouin authored and mergify[bot] committed Oct 4, 2022
1 parent ed56c0b commit c79bc89
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 14 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (apps/27-interchain-accounts) [\#2058](https://github.com/cosmos/ibc-go/pull/2058) Added `MessageRouter` interface and replaced `*baseapp.MsgServiceRouter` with it. The controller and host keepers of apps/27-interchain-accounts have been updated to use it.
* (apps/27-interchain-accounts)[\#2302](https://github.com/cosmos/ibc-go/pull/2302) Handle unwrapping of channel version in interchain accounts channel reopening handshake flow. The `host` submodule `Keeper` now requires an `ICS4Wrapper` similarly to the `controller` submodule.

* (transfer) [\#2377](https://github.com/cosmos/ibc-go/pull/2377) Adding `sequence` to `MsgTransferResponse`.

### Improvements

* (27-interchain-accounts) [\#1352](https://github.com/cosmos/ibc-go/issues/1352) Add support for Cosmos-SDK simulation to ics27 module.
Expand Down
5 changes: 5 additions & 0 deletions docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -2158,6 +2158,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 packet on the channel. |





Expand Down
9 changes: 7 additions & 2 deletions modules/apps/transfer/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,14 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
return nil, err
}

<<<<<<< HEAD
if err := k.SendTransfer(
=======
sequence, err := k.sendTransfer(
>>>>>>> 3363917 (MsgTransferResponse add sequence (#2377))
ctx, msg.SourcePort, msg.SourceChannel, msg.Token, sender, msg.Receiver, msg.TimeoutHeight, msg.TimeoutTimestamp,
); err != nil {
)
if err != nil {
return nil, err
}

Expand All @@ -39,5 +44,5 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
),
})

return &types.MsgTransferResponse{}, nil
return &types.MsgTransferResponse{Sequence: sequence}, nil
}
1 change: 1 addition & 0 deletions modules/apps/transfer/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
47 changes: 37 additions & 10 deletions modules/apps/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,44 @@ func (k Keeper) SendTransfer(
timeoutHeight clienttypes.Height,
timeoutTimestamp uint64,
) error {
<<<<<<< HEAD
=======
_, 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) {
>>>>>>> 3363917 (MsgTransferResponse add sequence (#2377))
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()
Expand All @@ -79,7 +106,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,
)
Expand All @@ -89,7 +116,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
Expand All @@ -102,7 +129,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
}
}

Expand All @@ -125,7 +152,7 @@ func (k Keeper) SendTransfer(
if err := k.bankKeeper.SendCoins(
ctx, sender, escrowAddress, sdk.NewCoins(token),
); err != nil {
return err
return 0, err
}

} else {
Expand All @@ -135,7 +162,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(
Expand Down Expand Up @@ -164,7 +191,7 @@ func (k Keeper) SendTransfer(
)

if err := k.ics4Wrapper.SendPacket(ctx, channelCap, packet); err != nil {
return err
return 0, err
}

defer func() {
Expand All @@ -183,7 +210,7 @@ func (k Keeper) SendTransfer(
)
}()

return nil
return sequence, nil
}

// OnRecvPacket processes a cross chain fungible token transfer. If the
Expand Down
72 changes: 71 additions & 1 deletion modules/apps/transfer/types/tx.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion proto/ibc/applications/transfer/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit c79bc89

Please sign in to comment.