Skip to content

Commit

Permalink
MsgTransferResponse add sequence (cosmos#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 cosmos#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
  • Loading branch information
agouin authored Oct 4, 2022
1 parent 8c80802 commit 3363917
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 47 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,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

* (modules/light-clients/07-tendermint) [\#1713](https://github.com/cosmos/ibc-go/pull/1713) Allow client upgrade proposals to update `TrustingPeriod`. See ADR-026 for context.
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 @@ -2267,6 +2267,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
7 changes: 4 additions & 3 deletions modules/apps/transfer/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,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
}

Expand All @@ -39,5 +40,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
25 changes: 13 additions & 12 deletions modules/apps/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (k Keeper) SendTransfer(
timeoutHeight clienttypes.Height,
timeoutTimestamp uint64,
) error {
return k.sendTransfer(
_, err := k.sendTransfer(
ctx,
sourcePort,
sourceChannel,
Expand All @@ -70,6 +70,7 @@ func (k Keeper) SendTransfer(
timeoutHeight,
timeoutTimestamp,
)
return err
}

// sendTransfer handles transfer sending logic.
Expand All @@ -82,18 +83,18 @@ func (k Keeper) sendTransfer(
receiver string,
timeoutHeight clienttypes.Height,
timeoutTimestamp uint64,
) error {
) (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()
Expand All @@ -102,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,
)
Expand All @@ -112,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
Expand All @@ -125,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
}
}

Expand All @@ -148,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 {
Expand All @@ -158,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(
Expand Down Expand Up @@ -187,7 +188,7 @@ func (k Keeper) sendTransfer(
)

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

defer func() {
Expand All @@ -206,7 +207,7 @@ func (k Keeper) sendTransfer(
)
}()

return nil
return sequence, nil
}

// OnRecvPacket processes a cross chain fungible token transfer. If the
Expand Down
99 changes: 68 additions & 31 deletions 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 3363917

Please sign in to comment.