diff --git a/CHANGELOG.md b/CHANGELOG.md index d9299281e19..00a5a5ba760 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (core) [\#6138](https://github.com/cosmos/ibc-go/pull/6138) Remove `Router` reference from IBC core keeper and use instead the router on the existing `PortKeeper` reference. * (core/04-channel) [\#6023](https://github.com/cosmos/ibc-go/pull/6023) Remove emission of non-hexlified event attributes `packet_data` and `packet_ack`. * (core) [\#6320](https://github.com/cosmos/ibc-go/pull/6320) Remove unnecessary `Proof` interface from `exported` package. +* (core/05-port) [\#6341](https://github.com/cosmos/ibc-go/pull/6341) Modify `UnmarshalPacketData` interface to take in the context, portID, and channelID. This allows for packet data's to be unmarshaled based on the channel version. ### State Machine Breaking diff --git a/docs/docs/05-migrations/13-v8-to-v9.md b/docs/docs/05-migrations/13-v8-to-v9.md index afbf6a0e98c..bd67a09515d 100644 --- a/docs/docs/05-migrations/13-v8-to-v9.md +++ b/docs/docs/05-migrations/13-v8-to-v9.md @@ -66,6 +66,7 @@ func AssertEvents( ### IBC core +- `UnmarshalPacketData` now takes in the context, portID, and channelID. This allows the packet data to be unmarshaled based on the channel version. - `Router` reference has been removed from IBC core keeper: [#6138](https://github.com/cosmos/ibc-go/pull/6138) ### ICS27 - Interchain Accounts diff --git a/e2e/tests/transfer/authz_test.go b/e2e/tests/transfer/authz_test.go index 446f27626cf..c2f803ae4ae 100644 --- a/e2e/tests/transfer/authz_test.go +++ b/e2e/tests/transfer/authz_test.go @@ -118,16 +118,18 @@ func (suite *AuthzTransferTestSuite) TestAuthz_MsgTransfer_Succeeds() { t.Run("broadcast MsgGrant", createMsgGrantFn) t.Run("broadcast MsgExec for ibc MsgTransfer", func(t *testing.T) { - transferMsg := transfertypes.MsgTransfer{ - SourcePort: channelA.PortID, - SourceChannel: channelA.ChannelID, - Token: testvalues.DefaultTransferAmount(chainADenom), - Sender: granterAddress, - Receiver: receiverWalletAddress, - TimeoutHeight: suite.GetTimeoutHeight(ctx, chainB), - } - - protoAny, err := codectypes.NewAnyWithValue(&transferMsg) + transferMsg := transfertypes.NewMsgTransfer( + channelA.PortID, + channelA.ChannelID, + testvalues.DefaultTransferCoins(chainADenom), + granterAddress, + receiverWalletAddress, + suite.GetTimeoutHeight(ctx, chainB), + 0, + "", + ) + + protoAny, err := codectypes.NewAnyWithValue(transferMsg) suite.Require().NoError(err) msgExec := &authz.MsgExec{ @@ -175,16 +177,18 @@ func (suite *AuthzTransferTestSuite) TestAuthz_MsgTransfer_Succeeds() { }) t.Run("exec unauthorized MsgTransfer", func(t *testing.T) { - transferMsg := transfertypes.MsgTransfer{ - SourcePort: channelA.PortID, - SourceChannel: channelA.ChannelID, - Token: testvalues.DefaultTransferAmount(chainADenom), - Sender: granterAddress, - Receiver: receiverWalletAddress, - TimeoutHeight: suite.GetTimeoutHeight(ctx, chainB), - } - - protoAny, err := codectypes.NewAnyWithValue(&transferMsg) + transferMsg := transfertypes.NewMsgTransfer( + channelA.PortID, + channelA.ChannelID, + testvalues.DefaultTransferCoins(chainADenom), + granterAddress, + receiverWalletAddress, + suite.GetTimeoutHeight(ctx, chainB), + 0, + "", + ) + + protoAny, err := codectypes.NewAnyWithValue(transferMsg) suite.Require().NoError(err) msgExec := &authz.MsgExec{ @@ -255,16 +259,18 @@ func (suite *AuthzTransferTestSuite) TestAuthz_InvalidTransferAuthorizations() { const invalidSpendAmount = spendLimit + 1 t.Run("broadcast MsgExec for ibc MsgTransfer", func(t *testing.T) { - transferMsg := transfertypes.MsgTransfer{ - SourcePort: channelA.PortID, - SourceChannel: channelA.ChannelID, - Token: sdk.Coin{Denom: chainADenom, Amount: sdkmath.NewInt(invalidSpendAmount)}, - Sender: granterAddress, - Receiver: receiverWalletAddress, - TimeoutHeight: suite.GetTimeoutHeight(ctx, chainB), - } - - protoAny, err := codectypes.NewAnyWithValue(&transferMsg) + transferMsg := transfertypes.NewMsgTransfer( + channelA.PortID, + channelA.ChannelID, + sdk.NewCoins(sdk.Coin{Denom: chainADenom, Amount: sdkmath.NewInt(invalidSpendAmount)}), + granterAddress, + receiverWalletAddress, + suite.GetTimeoutHeight(ctx, chainB), + 0, + "", + ) + + protoAny, err := codectypes.NewAnyWithValue(transferMsg) suite.Require().NoError(err) msgExec := &authz.MsgExec{ @@ -312,16 +318,18 @@ func (suite *AuthzTransferTestSuite) TestAuthz_InvalidTransferAuthorizations() { invalidWalletAddress := invalidWallet.FormattedAddress() t.Run("broadcast MsgExec for ibc MsgTransfer", func(t *testing.T) { - transferMsg := transfertypes.MsgTransfer{ - SourcePort: channelA.PortID, - SourceChannel: channelA.ChannelID, - Token: sdk.Coin{Denom: chainADenom, Amount: sdkmath.NewInt(spendLimit)}, - Sender: granterAddress, - Receiver: invalidWalletAddress, - TimeoutHeight: suite.GetTimeoutHeight(ctx, chainB), - } - - protoAny, err := codectypes.NewAnyWithValue(&transferMsg) + transferMsg := transfertypes.NewMsgTransfer( + channelA.PortID, + channelA.ChannelID, + sdk.NewCoins(sdk.Coin{Denom: chainADenom, Amount: sdkmath.NewInt(spendLimit)}), + granterAddress, + invalidWalletAddress, + suite.GetTimeoutHeight(ctx, chainB), + 0, + "", + ) + + protoAny, err := codectypes.NewAnyWithValue(transferMsg) suite.Require().NoError(err) msgExec := &authz.MsgExec{ diff --git a/e2e/tests/transfer/incentivized_test.go b/e2e/tests/transfer/incentivized_test.go index 5535f9d37ae..cbf9ac79c99 100644 --- a/e2e/tests/transfer/incentivized_test.go +++ b/e2e/tests/transfer/incentivized_test.go @@ -198,7 +198,7 @@ func (s *IncentivizedTransferTestSuite) TestMsgPayPacketFee_InvalidReceiverAccou transferAmount := testvalues.DefaultTransferAmount(chainADenom) t.Run("send IBC transfer", func(t *testing.T) { - msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, transferAmount, chainAWallet.FormattedAddress(), testvalues.InvalidAddress, s.GetTimeoutHeight(ctx, chainB), 0, "") + msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, sdk.NewCoins(transferAmount), chainAWallet.FormattedAddress(), testvalues.InvalidAddress, s.GetTimeoutHeight(ctx, chainB), 0, "") txResp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgTransfer) // this message should be successful, as receiver account is not validated on the sending chain. s.AssertTxSuccess(txResp) @@ -323,7 +323,7 @@ func (s *IncentivizedTransferTestSuite) TestMultiMsg_MsgPayPacketFeeSingleSender }) msgPayPacketFee := feetypes.NewMsgPayPacketFee(testFee, channelA.PortID, channelA.ChannelID, chainAWallet.FormattedAddress(), nil) - msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, transferAmount, chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") + msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, sdk.NewCoins(transferAmount), chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") resp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgPayPacketFee, msgTransfer) s.AssertTxSuccess(resp) diff --git a/e2e/tests/transfer/localhost_test.go b/e2e/tests/transfer/localhost_test.go index 498782df5e3..fac88272018 100644 --- a/e2e/tests/transfer/localhost_test.go +++ b/e2e/tests/transfer/localhost_test.go @@ -70,7 +70,7 @@ func (s *LocalhostTransferTestSuite) TestMsgTransfer_Localhost() { t.Run("channel open init localhost", func(t *testing.T) { msgChanOpenInit := channeltypes.NewMsgChannelOpenInit( - transfertypes.PortID, transfertypes.Version, + transfertypes.PortID, transfertypes.V2, channeltypes.UNORDERED, []string{exported.LocalhostConnectionID}, transfertypes.PortID, rlyWallet.FormattedAddress(), ) @@ -85,10 +85,10 @@ func (s *LocalhostTransferTestSuite) TestMsgTransfer_Localhost() { t.Run("channel open try localhost", func(t *testing.T) { msgChanOpenTry := channeltypes.NewMsgChannelOpenTry( - transfertypes.PortID, transfertypes.Version, + transfertypes.PortID, transfertypes.V2, channeltypes.UNORDERED, []string{exported.LocalhostConnectionID}, transfertypes.PortID, msgChanOpenInitRes.ChannelId, - transfertypes.Version, localhost.SentinelProof, clienttypes.ZeroHeight(), rlyWallet.FormattedAddress(), + transfertypes.V2, localhost.SentinelProof, clienttypes.ZeroHeight(), rlyWallet.FormattedAddress(), ) txResp := s.BroadcastMessages(ctx, chainA, rlyWallet, msgChanOpenTry) @@ -100,7 +100,7 @@ func (s *LocalhostTransferTestSuite) TestMsgTransfer_Localhost() { t.Run("channel open ack localhost", func(t *testing.T) { msgChanOpenAck := channeltypes.NewMsgChannelOpenAck( transfertypes.PortID, msgChanOpenInitRes.ChannelId, - msgChanOpenTryRes.ChannelId, transfertypes.Version, + msgChanOpenTryRes.ChannelId, transfertypes.V2, localhost.SentinelProof, clienttypes.ZeroHeight(), rlyWallet.FormattedAddress(), ) diff --git a/e2e/tests/transfer/upgrades_test.go b/e2e/tests/transfer/upgrades_test.go index 2457590d0c6..c32b324fdca 100644 --- a/e2e/tests/transfer/upgrades_test.go +++ b/e2e/tests/transfer/upgrades_test.go @@ -13,6 +13,8 @@ import ( sdkmath "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/ibc-go/e2e/testsuite" "github.com/cosmos/ibc-go/e2e/testsuite/query" "github.com/cosmos/ibc-go/e2e/testvalues" @@ -196,7 +198,7 @@ func (s *TransferChannelUpgradesTestSuite) TestChannelUpgrade_WithFeeMiddleware_ transferAmount := testvalues.DefaultTransferAmount(chainA.Config().Denom) msgPayPacketFee := feetypes.NewMsgPayPacketFee(testFee, channelA.PortID, channelA.ChannelID, chainAWallet.FormattedAddress(), nil) - msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, transferAmount, chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") + msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, sdk.NewCoins(transferAmount), chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") resp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgPayPacketFee, msgTransfer) s.AssertTxSuccess(resp) }) @@ -360,7 +362,7 @@ func (s *TransferChannelUpgradesTestSuite) TestChannelUpgrade_WithFeeMiddleware_ s.Require().NoError(err) s.Require().Equal(channeltypes.OPEN, channel.State, "the channel state is not OPEN") - s.Require().Equal(transfertypes.Version, channel.Version, "the channel version is not ics20-1") + s.Require().Equal(transfertypes.V2, channel.Version, "the channel version is not ics20-2") errorReceipt, err := query.UpgradeError(ctx, chainA, channelA.PortID, channelA.ChannelID) s.Require().NoError(err) @@ -373,7 +375,7 @@ func (s *TransferChannelUpgradesTestSuite) TestChannelUpgrade_WithFeeMiddleware_ s.Require().NoError(err) s.Require().Equal(channeltypes.OPEN, channel.State, "the channel state is not OPEN") - s.Require().Equal(transfertypes.Version, channel.Version, "the channel version is not ics20-1") + s.Require().Equal(transfertypes.V2, channel.Version, "the channel version is not ics20-2") errorReceipt, err := query.UpgradeError(ctx, chainB, channelB.PortID, channelB.ChannelID) s.Require().NoError(err) diff --git a/e2e/tests/upgrades/upgrade_test.go b/e2e/tests/upgrades/upgrade_test.go index c023fe3a7c8..ff536fa24f6 100644 --- a/e2e/tests/upgrades/upgrade_test.go +++ b/e2e/tests/upgrades/upgrade_test.go @@ -957,7 +957,7 @@ func (s *UpgradeTestSuite) TestV8ToV8_1ChainUpgrade_ChannelUpgrades() { transferAmount := testvalues.DefaultTransferAmount(chainA.Config().Denom) msgPayPacketFee := feetypes.NewMsgPayPacketFee(testFee, channelA.PortID, channelA.ChannelID, chainAWallet.FormattedAddress(), nil) - msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, transferAmount, chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") + msgTransfer := transfertypes.NewMsgTransfer(channelA.PortID, channelA.ChannelID, sdk.NewCoins(transferAmount), chainAWallet.FormattedAddress(), chainBWallet.FormattedAddress(), s.GetTimeoutHeight(ctx, chainB), 0, "") resp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgPayPacketFee, msgTransfer) s.AssertTxSuccess(resp) }) diff --git a/e2e/testsuite/testsuite.go b/e2e/testsuite/testsuite.go index 1e6bd840ad3..4552dea20ee 100644 --- a/e2e/testsuite/testsuite.go +++ b/e2e/testsuite/testsuite.go @@ -154,6 +154,10 @@ func (s *E2ETestSuite) ConfigureRelayer(ctx context.Context, chainA, chainB ibc. pathName := s.generatePathName() channelOptions := ibc.DefaultChannelOpts() + // For now, set the version to the latest transfer module version + // DefaultChannelOpts uses V1 at the moment + channelOptions.Version = transfertypes.V2 + if channelOpts != nil { channelOpts(&channelOptions) } @@ -449,7 +453,7 @@ func (s *E2ETestSuite) GetRelayerExecReporter() *testreporter.RelayerExecReporte // TransferChannelOptions configures both of the chains to have non-incentivized transfer channels. func (*E2ETestSuite) TransferChannelOptions() func(options *ibc.CreateChannelOptions) { return func(opts *ibc.CreateChannelOptions) { - opts.Version = transfertypes.Version + opts.Version = transfertypes.V2 opts.SourcePortName = transfertypes.PortID opts.DestPortName = transfertypes.PortID } @@ -459,7 +463,7 @@ func (*E2ETestSuite) TransferChannelOptions() func(options *ibc.CreateChannelOpt func (s *E2ETestSuite) FeeMiddlewareChannelOptions() func(options *ibc.CreateChannelOptions) { versionMetadata := feetypes.Metadata{ FeeVersion: feetypes.Version, - AppVersion: transfertypes.Version, + AppVersion: transfertypes.V2, } versionBytes, err := feetypes.ModuleCdc.MarshalJSON(&versionMetadata) s.Require().NoError(err) diff --git a/e2e/testsuite/tx.go b/e2e/testsuite/tx.go index 02d861dc45a..6ad082e2635 100644 --- a/e2e/testsuite/tx.go +++ b/e2e/testsuite/tx.go @@ -280,7 +280,7 @@ func (s *E2ETestSuite) ExecuteGovV1Beta1Proposal(ctx context.Context, chain ibc. func (s *E2ETestSuite) Transfer(ctx context.Context, chain ibc.Chain, user ibc.Wallet, portID, channelID string, token sdk.Coin, sender, receiver string, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, memo string, ) sdk.TxResponse { - msg := transfertypes.NewMsgTransfer(portID, channelID, token, sender, receiver, timeoutHeight, timeoutTimestamp, memo) + msg := transfertypes.NewMsgTransfer(portID, channelID, sdk.NewCoins(token), sender, receiver, timeoutHeight, timeoutTimestamp, memo) return s.BroadcastMessages(ctx, chain, user, msg) } diff --git a/e2e/testvalues/values.go b/e2e/testvalues/values.go index 01dae17234c..d5cd8a9c968 100644 --- a/e2e/testvalues/values.go +++ b/e2e/testvalues/values.go @@ -43,6 +43,10 @@ func DefaultTransferAmount(denom string) sdk.Coin { return sdk.Coin{Denom: denom, Amount: sdkmath.NewInt(IBCTransferAmount)} } +func DefaultTransferCoins(denom string) sdk.Coins { + return sdk.NewCoins(DefaultTransferAmount(denom)) +} + func TransferAmount(amount int64, denom string) sdk.Coin { return sdk.Coin{Denom: denom, Amount: sdkmath.NewInt(amount)} } diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index fe9ef65a026..dc27eeda2d8 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -340,7 +340,7 @@ func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) // UnmarshalPacketData attempts to unmarshal the provided packet data bytes // into an InterchainAccountPacketData. This function implements the optional // PacketDataUnmarshaler interface required for ADR 008 support. -func (IBCMiddleware) UnmarshalPacketData(bz []byte) (interface{}, error) { +func (IBCMiddleware) UnmarshalPacketData(_ sdk.Context, _, _ string, bz []byte) (interface{}, error) { var data icatypes.InterchainAccountPacketData err := data.UnmarshalJSON(bz) if err != nil { diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index 95a25ac65a9..e9ab95cc1b9 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -1276,13 +1276,15 @@ func (suite *InterchainAccountsTestSuite) TestPacketDataUnmarshalerInterface() { Memo: "", } - packetData, err := controller.IBCMiddleware{}.UnmarshalPacketData(expPacketData.GetBytes()) + // Context, port identifier and channel identifier are unused for controller. + packetData, err := controller.IBCMiddleware{}.UnmarshalPacketData(suite.chainA.GetContext(), "", "", expPacketData.GetBytes()) suite.Require().NoError(err) suite.Require().Equal(expPacketData, packetData) // test invalid packet data invalidPacketData := []byte("invalid packet data") - packetData, err = controller.IBCMiddleware{}.UnmarshalPacketData(invalidPacketData) + // Context, port identifier and channel identifier are not used for controller. + packetData, err = controller.IBCMiddleware{}.UnmarshalPacketData(suite.chainA.GetContext(), "", "", invalidPacketData) suite.Require().Error(err) suite.Require().Nil(packetData) } diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go index 3829a4c9acd..099e51e92cd 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module.go @@ -183,7 +183,7 @@ func (IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, pr // UnmarshalPacketData attempts to unmarshal the provided packet data bytes // into an InterchainAccountPacketData. This function implements the optional // PacketDataUnmarshaler interface required for ADR 008 support. -func (IBCModule) UnmarshalPacketData(bz []byte) (interface{}, error) { +func (IBCModule) UnmarshalPacketData(_ sdk.Context, _, _ string, bz []byte) (interface{}, error) { var data icatypes.InterchainAccountPacketData err := data.UnmarshalJSON(bz) if err != nil { diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index 98dbb2b36a0..9ffb67a8f4d 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -883,13 +883,15 @@ func (suite *InterchainAccountsTestSuite) TestPacketDataUnmarshalerInterface() { Memo: "", } - packetData, err := icahost.IBCModule{}.UnmarshalPacketData(expPacketData.GetBytes()) + // Context, port identifier and channel identifier are unused for host. + packetData, err := icahost.IBCModule{}.UnmarshalPacketData(suite.chainA.GetContext(), "", "", expPacketData.GetBytes()) suite.Require().NoError(err) suite.Require().Equal(expPacketData, packetData) // test invalid packet data invalidPacketData := []byte("invalid packet data") - packetData, err = icahost.IBCModule{}.UnmarshalPacketData(invalidPacketData) + // Context, port identifier and channel identifier are unused for host. + packetData, err = icahost.IBCModule{}.UnmarshalPacketData(suite.chainA.GetContext(), "", "", invalidPacketData) suite.Require().Error(err) suite.Require().Nil(packetData) } diff --git a/modules/apps/27-interchain-accounts/host/keeper/relay_test.go b/modules/apps/27-interchain-accounts/host/keeper/relay_test.go index ac5d41378e3..f49e623e939 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/relay_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/relay_test.go @@ -342,15 +342,16 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { interchainAccountAddr, found := suite.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID) suite.Require().True(found) - msg := &transfertypes.MsgTransfer{ - SourcePort: transferPath.EndpointA.ChannelConfig.PortID, - SourceChannel: transferPath.EndpointA.ChannelID, - Token: sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100)), - Sender: interchainAccountAddr, - Receiver: suite.chainA.SenderAccount.GetAddress().String(), - TimeoutHeight: suite.chainB.GetTimeoutHeight(), - TimeoutTimestamp: uint64(0), - } + msg := transfertypes.NewMsgTransfer( + transferPath.EndpointA.ChannelConfig.PortID, + transferPath.EndpointA.ChannelID, + sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100))), + interchainAccountAddr, + suite.chainA.SenderAccount.GetAddress().String(), + suite.chainB.GetTimeoutHeight(), + 0, + "", + ) data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}, encoding) suite.Require().NoError(err) @@ -376,15 +377,16 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { interchainAccountAddr, found := suite.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID) suite.Require().True(found) - msg := &transfertypes.MsgTransfer{ - SourcePort: transferPath.EndpointA.ChannelConfig.PortID, - SourceChannel: transferPath.EndpointA.ChannelID, - Token: sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100)), - Sender: interchainAccountAddr, - Receiver: "", - TimeoutHeight: suite.chainB.GetTimeoutHeight(), - TimeoutTimestamp: uint64(0), - } + msg := transfertypes.NewMsgTransfer( + transferPath.EndpointA.ChannelConfig.PortID, + transferPath.EndpointA.ChannelID, + sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100))), + interchainAccountAddr, + "", + suite.chainB.GetTimeoutHeight(), + 0, + "", + ) data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}, encoding) suite.Require().NoError(err) diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go index 218efb08882..671206eab8f 100644 --- a/modules/apps/29-fee/ibc_middleware.go +++ b/modules/apps/29-fee/ibc_middleware.go @@ -472,11 +472,11 @@ func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) // UnmarshalPacketData attempts to use the underlying app to unmarshal the packet data. // If the underlying app does not support the PacketDataUnmarshaler interface, an error is returned. // This function implements the optional PacketDataUnmarshaler interface required for ADR 008 support. -func (im IBCMiddleware) UnmarshalPacketData(bz []byte) (interface{}, error) { +func (im IBCMiddleware) UnmarshalPacketData(ctx sdk.Context, portID, channelID string, bz []byte) (interface{}, error) { unmarshaler, ok := im.app.(porttypes.PacketDataUnmarshaler) if !ok { return nil, errorsmod.Wrapf(types.ErrUnsupportedAction, "underlying app does not implement %T", (*porttypes.PacketDataUnmarshaler)(nil)) } - return unmarshaler.UnmarshalPacketData(bz) + return unmarshaler.UnmarshalPacketData(ctx, portID, channelID, bz) } diff --git a/modules/apps/29-fee/ibc_middleware_test.go b/modules/apps/29-fee/ibc_middleware_test.go index fc16fd900fc..dc59f277d3f 100644 --- a/modules/apps/29-fee/ibc_middleware_test.go +++ b/modules/apps/29-fee/ibc_middleware_test.go @@ -1573,7 +1573,8 @@ func (suite *FeeTestSuite) TestPacketDataUnmarshalerInterface() { feeModule, ok := cbs.(porttypes.PacketDataUnmarshaler) suite.Require().True(ok) - packetData, err := feeModule.UnmarshalPacketData(ibcmock.MockPacketData) + // Context, port identifier, channel identifier are not used in current wiring of fee. + packetData, err := feeModule.UnmarshalPacketData(suite.chainA.GetContext(), "", "", ibcmock.MockPacketData) suite.Require().NoError(err) suite.Require().Equal(ibcmock.MockPacketData, packetData) } @@ -1582,7 +1583,8 @@ func (suite *FeeTestSuite) TestPacketDataUnmarshalerInterfaceError() { // test the case when the underlying application cannot be casted to a PacketDataUnmarshaler mockFeeMiddleware := ibcfee.NewIBCMiddleware(nil, feekeeper.Keeper{}) - _, err := mockFeeMiddleware.UnmarshalPacketData(ibcmock.MockPacketData) + // Context, port identifier, channel identifier are not used in mockFeeMiddleware. + _, err := mockFeeMiddleware.UnmarshalPacketData(suite.chainA.GetContext(), "", "", ibcmock.MockPacketData) expError := errorsmod.Wrapf(types.ErrUnsupportedAction, "underlying app does not implement %T", (*porttypes.PacketDataUnmarshaler)(nil)) suite.Require().ErrorIs(err, expError) } diff --git a/modules/apps/29-fee/keeper/events_test.go b/modules/apps/29-fee/keeper/events_test.go index f4f49abcf0d..38b64cadd80 100644 --- a/modules/apps/29-fee/keeper/events_test.go +++ b/modules/apps/29-fee/keeper/events_test.go @@ -93,7 +93,7 @@ func (suite *KeeperTestSuite) TestIncentivizePacketEvent() { func (suite *KeeperTestSuite) TestDistributeFeeEvent() { // create an incentivized transfer path path := ibctesting.NewPath(suite.chainA, suite.chainB) - feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version})) + feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.V2})) path.EndpointA.ChannelConfig.Version = feeTransferVersion path.EndpointB.ChannelConfig.Version = feeTransferVersion path.EndpointA.ChannelConfig.PortID = transfertypes.PortID @@ -113,7 +113,7 @@ func (suite *KeeperTestSuite) TestDistributeFeeEvent() { msgTransfer := transfertypes.NewMsgTransfer( path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100)), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), + sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100))), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, "", ) diff --git a/modules/apps/29-fee/transfer_test.go b/modules/apps/29-fee/transfer_test.go index 5281927d963..dcfa344ab87 100644 --- a/modules/apps/29-fee/transfer_test.go +++ b/modules/apps/29-fee/transfer_test.go @@ -12,7 +12,7 @@ import ( // Integration test to ensure ics29 works with ics20 func (suite *FeeTestSuite) TestFeeTransfer() { path := ibctesting.NewPath(suite.chainA, suite.chainB) - feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version})) + feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.V2})) path.EndpointA.ChannelConfig.Version = feeTransferVersion path.EndpointB.ChannelConfig.Version = feeTransferVersion path.EndpointA.ChannelConfig.PortID = transfertypes.PortID @@ -30,7 +30,7 @@ func (suite *FeeTestSuite) TestFeeTransfer() { msgs := []sdk.Msg{ types.NewMsgPayPacketFee(fee, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, suite.chainA.SenderAccount.GetAddress().String(), nil), - transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, coin, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, ""), + transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.NewCoins(coin), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, ""), } res, err := suite.chainA.SendMsgs(msgs...) suite.Require().NoError(err) // message committed @@ -94,13 +94,13 @@ func (suite *FeeTestSuite) TestTransferFeeUpgrade() { // configure the initial path to create a regular transfer channel path.EndpointA.ChannelConfig.PortID = transfertypes.PortID path.EndpointB.ChannelConfig.PortID = transfertypes.PortID - path.EndpointA.ChannelConfig.Version = transfertypes.Version - path.EndpointB.ChannelConfig.Version = transfertypes.Version + path.EndpointA.ChannelConfig.Version = transfertypes.V2 + path.EndpointB.ChannelConfig.Version = transfertypes.V2 path.Setup() // configure the channel upgrade to upgrade to an incentivized fee enabled transfer channel - upgradeVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version})) + upgradeVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.V2})) path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = upgradeVersion path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = upgradeVersion @@ -138,7 +138,7 @@ func (suite *FeeTestSuite) TestTransferFeeUpgrade() { fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee) msgs := []sdk.Msg{ types.NewMsgPayPacketFee(fee, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, suite.chainA.SenderAccount.GetAddress().String(), nil), - transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, ibctesting.TestCoin, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, ""), + transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.NewCoins(ibctesting.TestCoin), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, ""), } res, err := suite.chainA.SendMsgs(msgs...) @@ -168,7 +168,7 @@ func (suite *FeeTestSuite) TestOnesidedFeeMiddlewareTransferHandshake() { RemoveFeeMiddleware(suite.chainB) // remove fee middleware from chainB path := ibctesting.NewPath(suite.chainA, suite.chainB) - feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version})) + feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.V2})) path.EndpointA.ChannelConfig.Version = feeTransferVersion // this will be renegotiated by the Try step path.EndpointB.ChannelConfig.Version = "" // this will be overwritten by the Try step path.EndpointA.ChannelConfig.PortID = transfertypes.PortID @@ -176,6 +176,6 @@ func (suite *FeeTestSuite) TestOnesidedFeeMiddlewareTransferHandshake() { path.Setup() - suite.Require().Equal(path.EndpointA.ChannelConfig.Version, transfertypes.Version) - suite.Require().Equal(path.EndpointB.ChannelConfig.Version, transfertypes.Version) + suite.Require().Equal(path.EndpointA.ChannelConfig.Version, transfertypes.V2) + suite.Require().Equal(path.EndpointB.ChannelConfig.Version, transfertypes.V2) } diff --git a/modules/apps/callbacks/callbacks_test.go b/modules/apps/callbacks/callbacks_test.go index 092dedd4bbd..f9874896a22 100644 --- a/modules/apps/callbacks/callbacks_test.go +++ b/modules/apps/callbacks/callbacks_test.go @@ -11,6 +11,7 @@ import ( "cosmossdk.io/log" sdkmath "cosmossdk.io/math" + storetypes "cosmossdk.io/store/types" simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" sdk "github.com/cosmos/cosmos-sdk/types" @@ -24,6 +25,7 @@ import ( icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types" feetypes "github.com/cosmos/ibc-go/v8/modules/apps/29-fee/types" transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types" ibctesting "github.com/cosmos/ibc-go/v8/testing" @@ -78,8 +80,8 @@ func (s *CallbacksTestSuite) SetupTransferTest() { s.path.EndpointA.ChannelConfig.PortID = ibctesting.TransferPort s.path.EndpointB.ChannelConfig.PortID = ibctesting.TransferPort - s.path.EndpointA.ChannelConfig.Version = transfertypes.Version - s.path.EndpointB.ChannelConfig.Version = transfertypes.Version + s.path.EndpointA.ChannelConfig.Version = transfertypes.V2 + s.path.EndpointB.ChannelConfig.Version = transfertypes.V2 s.path.Setup() } @@ -88,7 +90,7 @@ func (s *CallbacksTestSuite) SetupTransferTest() { func (s *CallbacksTestSuite) SetupFeeTransferTest() { s.setupChains() - feeTransferVersion := string(feetypes.ModuleCdc.MustMarshalJSON(&feetypes.Metadata{FeeVersion: feetypes.Version, AppVersion: transfertypes.Version})) + feeTransferVersion := string(feetypes.ModuleCdc.MustMarshalJSON(&feetypes.Metadata{FeeVersion: feetypes.Version, AppVersion: transfertypes.V2})) s.path.EndpointA.ChannelConfig.Version = feeTransferVersion s.path.EndpointB.ChannelConfig.Version = feeTransferVersion s.path.EndpointA.ChannelConfig.PortID = transfertypes.PortID @@ -291,17 +293,24 @@ func (s *CallbacksTestSuite) AssertHasExecutedExpectedCallbackWithFee( // GetExpectedEvent returns the expected event for a callback. func GetExpectedEvent( - packetDataUnmarshaler porttypes.PacketDataUnmarshaler, remainingGas uint64, data []byte, srcPortID, + ctx sdk.Context, packetDataUnmarshaler porttypes.PacketDataUnmarshaler, remainingGas uint64, data []byte, srcPortID, eventPortID, eventChannelID string, seq uint64, callbackType types.CallbackType, expError error, ) (abci.Event, bool) { var ( callbackData types.CallbackData err error ) + + // Set up gas meter with remainingGas. + gasMeter := storetypes.NewGasMeter(remainingGas) + ctx = ctx.WithGasMeter(gasMeter) + if callbackType == types.CallbackTypeReceivePacket { - callbackData, err = types.GetDestCallbackData(packetDataUnmarshaler, data, srcPortID, remainingGas, maxCallbackGas) + packet := channeltypes.NewPacket(data, seq, "", "", eventPortID, eventChannelID, clienttypes.ZeroHeight(), 0) + callbackData, err = types.GetDestCallbackData(ctx, packetDataUnmarshaler, packet, maxCallbackGas) } else { - callbackData, err = types.GetSourceCallbackData(packetDataUnmarshaler, data, srcPortID, remainingGas, maxCallbackGas) + packet := channeltypes.NewPacket(data, seq, eventPortID, eventChannelID, "", "", clienttypes.ZeroHeight(), 0) + callbackData, err = types.GetSourceCallbackData(ctx, packetDataUnmarshaler, packet, maxCallbackGas) } if err != nil { return abci.Event{}, false diff --git a/modules/apps/callbacks/ibc_middleware.go b/modules/apps/callbacks/ibc_middleware.go index 0ab1f5f9887..816305913d8 100644 --- a/modules/apps/callbacks/ibc_middleware.go +++ b/modules/apps/callbacks/ibc_middleware.go @@ -99,7 +99,10 @@ func (im IBCMiddleware) SendPacket( return 0, err } - callbackData, err := types.GetSourceCallbackData(im.app, data, sourcePort, ctx.GasMeter().GasRemaining(), im.maxCallbackGas) + // packet is created withouth destination information present, GetSourceCallbackData does not use these. + packet := channeltypes.NewPacket(data, seq, sourcePort, sourceChannel, "", "", timeoutHeight, timeoutTimestamp) + + callbackData, err := types.GetSourceCallbackData(ctx, im.app, packet, im.maxCallbackGas) // SendPacket is not blocked if the packet does not opt-in to callbacks if err != nil { return seq, nil @@ -138,7 +141,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket( } callbackData, err := types.GetSourceCallbackData( - im.app, packet.GetData(), packet.GetSourcePort(), ctx.GasMeter().GasRemaining(), im.maxCallbackGas, + ctx, im.app, packet, im.maxCallbackGas, ) // OnAcknowledgementPacket is not blocked if the packet does not opt-in to callbacks if err != nil { @@ -172,7 +175,7 @@ func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Pac } callbackData, err := types.GetSourceCallbackData( - im.app, packet.GetData(), packet.GetSourcePort(), ctx.GasMeter().GasRemaining(), im.maxCallbackGas, + ctx, im.app, packet, im.maxCallbackGas, ) // OnTimeoutPacket is not blocked if the packet does not opt-in to callbacks if err != nil { @@ -208,7 +211,7 @@ func (im IBCMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet } callbackData, err := types.GetDestCallbackData( - im.app, packet.GetData(), packet.GetSourcePort(), ctx.GasMeter().GasRemaining(), im.maxCallbackGas, + ctx, im.app, packet, im.maxCallbackGas, ) // OnRecvPacket is not blocked if the packet does not opt-in to callbacks if err != nil { @@ -245,8 +248,13 @@ func (im IBCMiddleware) WriteAcknowledgement( return err } + chanPacket, ok := packet.(channeltypes.Packet) + if !ok { + panic(fmt.Errorf("expected type %T, got %T", &channeltypes.Packet{}, packet)) + } + callbackData, err := types.GetDestCallbackData( - im.app, packet.GetData(), packet.GetSourcePort(), ctx.GasMeter().GasRemaining(), im.maxCallbackGas, + ctx, im.app, chanPacket, im.maxCallbackGas, ) // WriteAcknowledgement is not blocked if the packet does not opt-in to callbacks if err != nil { @@ -417,6 +425,6 @@ func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) // UnmarshalPacketData defers to the underlying app to unmarshal the packet data. // This function implements the optional PacketDataUnmarshaler interface. -func (im IBCMiddleware) UnmarshalPacketData(bz []byte) (interface{}, error) { - return im.app.UnmarshalPacketData(bz) +func (im IBCMiddleware) UnmarshalPacketData(ctx sdk.Context, portID, channelID string, bz []byte) (interface{}, error) { + return im.app.UnmarshalPacketData(ctx, portID, channelID, bz) } diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index d4419ffd8ec..7c394bcaa9f 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/modules/apps/callbacks/ibc_middleware_test.go @@ -93,7 +93,7 @@ func (s *CallbacksTestSuite) TestWithICS4Wrapper() { } func (s *CallbacksTestSuite) TestSendPacket() { - var packetData transfertypes.FungibleTokenPacketData + var packetData transfertypes.FungibleTokenPacketDataV2 testCases := []struct { name string @@ -164,9 +164,17 @@ func (s *CallbacksTestSuite) TestSendPacket() { transferICS4Wrapper := GetSimApp(s.chainA).TransferKeeper.GetICS4Wrapper() - packetData = transfertypes.NewFungibleTokenPacketData( - ibctesting.TestCoin.GetDenom(), ibctesting.TestCoin.Amount.String(), ibctesting.TestAccAddress, - ibctesting.TestAccAddress, fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, simapp.SuccessContract), + packetData = transfertypes.NewFungibleTokenPacketDataV2( + []transfertypes.Token{ + { + Denom: ibctesting.TestCoin.GetDenom(), + Amount: ibctesting.TestCoin.Amount.String(), + Trace: []string{}, + }, + }, + ibctesting.TestAccAddress, + ibctesting.TestAccAddress, + fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, simapp.SuccessContract), ) chanCap := s.path.EndpointA.Chain.GetChannelCapability(s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID) @@ -192,7 +200,7 @@ func (s *CallbacksTestSuite) TestSendPacket() { s.Require().Equal(uint64(1), seq) expEvent, exists := GetExpectedEvent( - transferICS4Wrapper.(porttypes.PacketDataUnmarshaler), gasLimit, packetData.GetBytes(), s.path.EndpointA.ChannelConfig.PortID, + ctx, transferICS4Wrapper.(porttypes.PacketDataUnmarshaler), gasLimit, packetData.GetBytes(), s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID, seq, types.CallbackTypeSendPacket, nil, ) if exists { @@ -222,7 +230,7 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() { ) var ( - packetData transfertypes.FungibleTokenPacketData + packetData transfertypes.FungibleTokenPacketDataV2 packet channeltypes.Packet ack []byte ctx sdk.Context @@ -298,8 +306,16 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() { s.SetupTransferTest() userGasLimit = 600000 - packetData = transfertypes.NewFungibleTokenPacketData( - ibctesting.TestCoin.GetDenom(), ibctesting.TestCoin.Amount.String(), ibctesting.TestAccAddress, ibctesting.TestAccAddress, + packetData = transfertypes.NewFungibleTokenPacketDataV2( + []transfertypes.Token{ + { + Denom: ibctesting.TestCoin.GetDenom(), + Amount: ibctesting.TestCoin.Amount.String(), + Trace: []string{}, + }, + }, + ibctesting.TestAccAddress, + ibctesting.TestAccAddress, fmt.Sprintf(`{"src_callback": {"address":"%s", "gas_limit":"%d"}}`, simapp.SuccessContract, userGasLimit), ) @@ -365,7 +381,7 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() { s.Require().Equal(uint8(1), sourceStatefulCounter) expEvent, exists := GetExpectedEvent( - transferStack.(porttypes.PacketDataUnmarshaler), gasLimit, packet.Data, packet.SourcePort, + ctx, transferStack.(porttypes.PacketDataUnmarshaler), gasLimit, packet.Data, packet.SourcePort, packet.SourcePort, packet.SourceChannel, packet.Sequence, types.CallbackTypeAcknowledgementPacket, nil, ) s.Require().True(exists) @@ -384,7 +400,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() { ) var ( - packetData transfertypes.FungibleTokenPacketData + packetData transfertypes.FungibleTokenPacketDataV2 packet channeltypes.Packet ctx sdk.Context ) @@ -407,7 +423,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() { packet.Data = []byte("invalid packet data") }, noExecution, - ibcerrors.ErrUnknownRequest, + ibcerrors.ErrInvalidType, }, { "success: no-op on callback data is not valid", @@ -463,7 +479,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() { timeoutTimestamp := uint64(s.chainB.GetContext().BlockTime().UnixNano()) msg := transfertypes.NewMsgTransfer( s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID, - ibctesting.TestCoin, s.chainA.SenderAccount.GetAddress().String(), + sdk.NewCoins(ibctesting.TestCoin), s.chainA.SenderAccount.GetAddress().String(), s.chainB.SenderAccount.GetAddress().String(), clienttypes.ZeroHeight(), timeoutTimestamp, fmt.Sprintf(`{"src_callback": {"address":"%s", "gas_limit":"%d"}}`, ibctesting.TestAccAddress, userGasLimit), // set user gas limit above panic level in mock contract keeper ) @@ -527,7 +543,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() { s.Require().Equal(uint8(2), sourceStatefulCounter) expEvent, exists := GetExpectedEvent( - transferStack.(porttypes.PacketDataUnmarshaler), gasLimit, packet.Data, packet.SourcePort, + ctx, transferStack.(porttypes.PacketDataUnmarshaler), gasLimit, packet.Data, packet.SourcePort, packet.SourcePort, packet.SourceChannel, packet.Sequence, types.CallbackTypeTimeoutPacket, nil, ) s.Require().True(exists) @@ -546,7 +562,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() { ) var ( - packetData transfertypes.FungibleTokenPacketData + packetData transfertypes.FungibleTokenPacketDataV2 packet channeltypes.Packet ctx sdk.Context userGasLimit uint64 @@ -623,8 +639,16 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() { // set user gas limit above panic level in mock contract keeper userGasLimit = 600_000 - packetData = transfertypes.NewFungibleTokenPacketData( - ibctesting.TestCoin.GetDenom(), ibctesting.TestCoin.Amount.String(), ibctesting.TestAccAddress, s.chainB.SenderAccount.GetAddress().String(), + packetData = transfertypes.NewFungibleTokenPacketDataV2( + []transfertypes.Token{ + { + Denom: ibctesting.TestCoin.GetDenom(), + Amount: ibctesting.TestCoin.Amount.String(), + Trace: []string{}, + }, + }, + ibctesting.TestAccAddress, + s.chainB.SenderAccount.GetAddress().String(), fmt.Sprintf(`{"dest_callback": {"address":"%s", "gas_limit":"%d"}}`, ibctesting.TestAccAddress, userGasLimit), ) @@ -688,7 +712,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() { s.Require().Equal(uint8(1), destStatefulCounter) expEvent, exists := GetExpectedEvent( - transferStack.(porttypes.PacketDataUnmarshaler), gasLimit, packet.Data, packet.SourcePort, + ctx, transferStack.(porttypes.PacketDataUnmarshaler), gasLimit, packet.Data, packet.SourcePort, packet.DestinationPort, packet.DestinationChannel, packet.Sequence, types.CallbackTypeReceivePacket, nil, ) s.Require().True(exists) @@ -700,7 +724,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() { func (s *CallbacksTestSuite) TestWriteAcknowledgement() { var ( - packetData transfertypes.FungibleTokenPacketData + packetData transfertypes.FungibleTokenPacketDataV2 packet channeltypes.Packet ctx sdk.Context ack ibcexported.Acknowledgement @@ -747,8 +771,16 @@ func (s *CallbacksTestSuite) TestWriteAcknowledgement() { s.SetupTransferTest() // set user gas limit above panic level in mock contract keeper - packetData = transfertypes.NewFungibleTokenPacketData( - ibctesting.TestCoin.GetDenom(), ibctesting.TestCoin.Amount.String(), ibctesting.TestAccAddress, s.chainB.SenderAccount.GetAddress().String(), + packetData = transfertypes.NewFungibleTokenPacketDataV2( + []transfertypes.Token{ + { + Denom: ibctesting.TestCoin.GetDenom(), + Amount: ibctesting.TestCoin.Amount.String(), + Trace: []string{}, + }, + }, + ibctesting.TestAccAddress, + s.chainB.SenderAccount.GetAddress().String(), fmt.Sprintf(`{"dest_callback": {"address":"%s", "gas_limit":"600000"}}`, ibctesting.TestAccAddress), ) @@ -782,7 +814,7 @@ func (s *CallbacksTestSuite) TestWriteAcknowledgement() { s.Require().NoError(err) expEvent, exists := GetExpectedEvent( - transferICS4Wrapper.(porttypes.PacketDataUnmarshaler), gasLimit, packet.Data, packet.SourcePort, + ctx, transferICS4Wrapper.(porttypes.PacketDataUnmarshaler), gasLimit, packet.Data, packet.SourcePort, packet.DestinationPort, packet.DestinationChannel, packet.Sequence, types.CallbackTypeReceivePacket, nil, ) if exists { @@ -939,8 +971,13 @@ func (s *CallbacksTestSuite) TestProcessCallback() { } } -func (s *CallbacksTestSuite) TestUnmarshalPacketData() { +func (s *CallbacksTestSuite) TestUnmarshalPacketDataV1() { s.setupChains() + s.path.EndpointA.ChannelConfig.PortID = ibctesting.TransferPort + s.path.EndpointB.ChannelConfig.PortID = ibctesting.TransferPort + s.path.EndpointA.ChannelConfig.Version = transfertypes.V1 + s.path.EndpointB.ChannelConfig.Version = transfertypes.V1 + s.path.Setup() // We will pass the function call down the transfer stack to the transfer module // transfer stack UnmarshalPacketData call order: callbacks -> fee -> transfer @@ -950,18 +987,69 @@ func (s *CallbacksTestSuite) TestUnmarshalPacketData() { unmarshalerStack, ok := transferStack.(types.CallbacksCompatibleModule) s.Require().True(ok) - expPacketData := transfertypes.FungibleTokenPacketData{ + expPacketDataICS20V1 := transfertypes.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: ibctesting.TestAccAddress, Receiver: ibctesting.TestAccAddress, Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}, "dest_callback": {"address":"%s"}}`, ibctesting.TestAccAddress, ibctesting.TestAccAddress), } - data := expPacketData.GetBytes() - packetData, err := unmarshalerStack.UnmarshalPacketData(data) + expPacketDataICS20V2 := transfertypes.FungibleTokenPacketDataV2{ + Tokens: []transfertypes.Token{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + Trace: nil, + }, + }, + Sender: ibctesting.TestAccAddress, + Receiver: ibctesting.TestAccAddress, + Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}, "dest_callback": {"address":"%s"}}`, ibctesting.TestAccAddress, ibctesting.TestAccAddress), + } + + portID := s.path.EndpointA.ChannelConfig.PortID + channelID := s.path.EndpointA.ChannelID + + // Unmarshal ICS20 v1 packet data into v2 packet data + data := expPacketDataICS20V1.GetBytes() + packetData, err := unmarshalerStack.UnmarshalPacketData(s.chainA.GetContext(), portID, channelID, data) + s.Require().NoError(err) + s.Require().Equal(expPacketDataICS20V2, packetData) +} + +func (s *CallbacksTestSuite) TestUnmarshalPacketDataV2() { + s.SetupTransferTest() + + // We will pass the function call down the transfer stack to the transfer module + // transfer stack UnmarshalPacketData call order: callbacks -> fee -> transfer + transferStack, ok := s.chainA.App.GetIBCKeeper().PortKeeper.Route(transfertypes.ModuleName) + s.Require().True(ok) + + unmarshalerStack, ok := transferStack.(types.CallbacksCompatibleModule) + s.Require().True(ok) + + expPacketDataICS20V2 := transfertypes.FungibleTokenPacketDataV2{ + Tokens: []transfertypes.Token{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + Trace: nil, + }, + }, + Sender: ibctesting.TestAccAddress, + Receiver: ibctesting.TestAccAddress, + Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}, "dest_callback": {"address":"%s"}}`, ibctesting.TestAccAddress, ibctesting.TestAccAddress), + } + + portID := s.path.EndpointA.ChannelConfig.PortID + channelID := s.path.EndpointA.ChannelID + + // Unmarshal ICS20 v2 packet data + data := expPacketDataICS20V2.GetBytes() + packetData, err := unmarshalerStack.UnmarshalPacketData(s.chainA.GetContext(), portID, channelID, data) s.Require().NoError(err) - s.Require().Equal(expPacketData, packetData) + s.Require().Equal(expPacketDataICS20V2, packetData) } func (s *CallbacksTestSuite) TestGetAppVersion() { diff --git a/modules/apps/callbacks/replay_test.go b/modules/apps/callbacks/replay_test.go index d23de002613..04a8e5900fb 100644 --- a/modules/apps/callbacks/replay_test.go +++ b/modules/apps/callbacks/replay_test.go @@ -326,7 +326,7 @@ func (s *CallbacksTestSuite) ExecuteFailedTransfer(memo string) { msg := transfertypes.NewMsgTransfer( s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID, - amount, + sdk.NewCoins(amount), s.chainA.SenderAccount.GetAddress().String(), s.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, memo, diff --git a/modules/apps/callbacks/transfer_test.go b/modules/apps/callbacks/transfer_test.go index 4d288e96bbc..16698f074e6 100644 --- a/modules/apps/callbacks/transfer_test.go +++ b/modules/apps/callbacks/transfer_test.go @@ -189,7 +189,7 @@ func (s *CallbacksTestSuite) ExecuteTransfer(memo string) { msg := transfertypes.NewMsgTransfer( s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID, - amount, + sdk.NewCoins(amount), s.chainA.SenderAccount.GetAddress().String(), s.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, memo, @@ -223,7 +223,7 @@ func (s *CallbacksTestSuite) ExecuteTransferTimeout(memo string) { msg := transfertypes.NewMsgTransfer( s.path.EndpointA.ChannelConfig.PortID, s.path.EndpointA.ChannelID, - amount, + sdk.NewCoins(amount), s.chainA.SenderAccount.GetAddress().String(), s.chainB.SenderAccount.GetAddress().String(), timeoutHeight, timeoutTimestamp, memo, diff --git a/modules/apps/callbacks/types/callbacks.go b/modules/apps/callbacks/types/callbacks.go index 03c4128fb31..b7eaabec93b 100644 --- a/modules/apps/callbacks/types/callbacks.go +++ b/modules/apps/callbacks/types/callbacks.go @@ -6,6 +6,9 @@ import ( errorsmod "cosmossdk.io/errors" + sdk "github.com/cosmos/cosmos-sdk/types" + + channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types" ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported" ) @@ -67,18 +70,31 @@ type CallbackData struct { // GetSourceCallbackData parses the packet data and returns the source callback data. func GetSourceCallbackData( + ctx sdk.Context, packetDataUnmarshaler porttypes.PacketDataUnmarshaler, - data []byte, srcPortID string, remainingGas uint64, maxGas uint64, + packet channeltypes.Packet, + maxGas uint64, ) (CallbackData, error) { - return getCallbackData(packetDataUnmarshaler, data, srcPortID, remainingGas, maxGas, SourceCallbackKey) + packetData, err := packetDataUnmarshaler.UnmarshalPacketData(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetData()) + if err != nil { + return CallbackData{}, errorsmod.Wrap(ErrCannotUnmarshalPacketData, err.Error()) + } + + return getCallbackData(packetData, packet.GetSourcePort(), ctx.GasMeter().GasRemaining(), maxGas, SourceCallbackKey) } // GetDestCallbackData parses the packet data and returns the destination callback data. func GetDestCallbackData( + ctx sdk.Context, packetDataUnmarshaler porttypes.PacketDataUnmarshaler, - data []byte, srcPortID string, remainingGas, maxGas uint64, + packet channeltypes.Packet, maxGas uint64, ) (CallbackData, error) { - return getCallbackData(packetDataUnmarshaler, data, srcPortID, remainingGas, maxGas, DestinationCallbackKey) + packetData, err := packetDataUnmarshaler.UnmarshalPacketData(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetData()) + if err != nil { + return CallbackData{}, errorsmod.Wrap(ErrCannotUnmarshalPacketData, err.Error()) + } + + return getCallbackData(packetData, packet.GetSourcePort(), ctx.GasMeter().GasRemaining(), maxGas, DestinationCallbackKey) } // getCallbackData parses the packet data and returns the callback data. @@ -86,16 +102,11 @@ func GetDestCallbackData( // The addressGetter and gasLimitGetter functions are used to retrieve the callback // address and gas limit from the callback data. func getCallbackData( - packetDataUnmarshaler porttypes.PacketDataUnmarshaler, - data []byte, srcPortID string, remainingGas, + packetData interface{}, + srcPortID string, + remainingGas, maxGas uint64, callbackKey string, ) (CallbackData, error) { - // unmarshal packet data - packetData, err := packetDataUnmarshaler.UnmarshalPacketData(data) - if err != nil { - return CallbackData{}, errorsmod.Wrap(ErrCannotUnmarshalPacketData, err.Error()) - } - packetDataProvider, ok := packetData.(ibcexported.PacketDataProvider) if !ok { return CallbackData{}, ErrNotPacketDataProvider diff --git a/modules/apps/callbacks/types/callbacks_test.go b/modules/apps/callbacks/types/callbacks_test.go index 4521afc0a41..be4098f9bc9 100644 --- a/modules/apps/callbacks/types/callbacks_test.go +++ b/modules/apps/callbacks/types/callbacks_test.go @@ -3,26 +3,27 @@ package types_test import ( "fmt" + storetypes "cosmossdk.io/store/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cometbft/cometbft/crypto/secp256k1" "github.com/cosmos/ibc-go/modules/apps/callbacks/types" - "github.com/cosmos/ibc-go/v8/modules/apps/transfer" transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" - porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types" + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/v8/testing" ibcmock "github.com/cosmos/ibc-go/v8/testing/mock" ) func (s *CallbacksTypesTestSuite) TestGetCallbackData() { var ( - sender = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() - receiver = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() - packetDataUnmarshaler porttypes.PacketDataUnmarshaler - packetData []byte - remainingGas uint64 - callbackKey string + sender = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() + receiver = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() + packetData interface{} + remainingGas uint64 + callbackKey string ) // max gas is 1_000_000 @@ -36,14 +37,13 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { "success: source callback", func() { remainingGas = 2_000_000 - expPacketData := transfertypes.FungibleTokenPacketData{ + packetData = transfertypes.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: sender, Receiver: receiver, Memo: fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, sender), } - packetData = expPacketData.GetBytes() }, types.CallbackData{ CallbackAddress: sender, @@ -57,15 +57,15 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { "success: destination callback", func() { callbackKey = types.DestinationCallbackKey + remainingGas = 2_000_000 - expPacketData := transfertypes.FungibleTokenPacketData{ + packetData = transfertypes.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: sender, Receiver: receiver, Memo: fmt.Sprintf(`{"dest_callback": {"address": "%s"}}`, sender), } - packetData = expPacketData.GetBytes() }, types.CallbackData{ CallbackAddress: sender, @@ -79,15 +79,15 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { "success: destination callback with 0 user defined gas limit", func() { callbackKey = types.DestinationCallbackKey + remainingGas = 2_000_000 - expPacketData := transfertypes.FungibleTokenPacketData{ + packetData = transfertypes.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: sender, Receiver: receiver, Memo: fmt.Sprintf(`{"dest_callback": {"address": "%s", "gas_limit":"0"}}`, sender), } - packetData = expPacketData.GetBytes() }, types.CallbackData{ CallbackAddress: sender, @@ -100,14 +100,13 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { { "success: source callback with gas limit < remaining gas < max gas", func() { - expPacketData := transfertypes.FungibleTokenPacketData{ + packetData = transfertypes.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: sender, Receiver: receiver, Memo: fmt.Sprintf(`{"src_callback": {"address": "%s", "gas_limit": "50000"}}`, sender), } - packetData = expPacketData.GetBytes() remainingGas = 100_000 }, @@ -123,14 +122,13 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { "success: source callback with remaining gas < gas limit < max gas", func() { remainingGas = 100_000 - expPacketData := transfertypes.FungibleTokenPacketData{ + packetData = transfertypes.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: sender, Receiver: receiver, Memo: fmt.Sprintf(`{"src_callback": {"address": "%s", "gas_limit": "200000"}}`, sender), } - packetData = expPacketData.GetBytes() }, types.CallbackData{ CallbackAddress: sender, @@ -144,14 +142,13 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { "success: source callback with remaining gas < max gas < gas limit", func() { remainingGas = 100_000 - expPacketData := transfertypes.FungibleTokenPacketData{ + packetData = transfertypes.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: sender, Receiver: receiver, Memo: fmt.Sprintf(`{"src_callback": {"address": "%s", "gas_limit": "2000000"}}`, sender), } - packetData = expPacketData.GetBytes() }, types.CallbackData{ CallbackAddress: sender, @@ -165,15 +162,15 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { "success: destination callback with remaining gas < max gas < gas limit", func() { callbackKey = types.DestinationCallbackKey + remainingGas = 100_000 - expPacketData := transfertypes.FungibleTokenPacketData{ + packetData = transfertypes.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: sender, Receiver: receiver, Memo: fmt.Sprintf(`{"dest_callback": {"address": "%s", "gas_limit": "2000000"}}`, sender), } - packetData = expPacketData.GetBytes() }, types.CallbackData{ CallbackAddress: sender, @@ -187,14 +184,13 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { "success: source callback with max gas < remaining gas < gas limit", func() { remainingGas = 2_000_000 - expPacketData := transfertypes.FungibleTokenPacketData{ + packetData = transfertypes.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: sender, Receiver: receiver, Memo: fmt.Sprintf(`{"src_callback": {"address": "%s", "gas_limit": "3000000"}}`, sender), } - packetData = expPacketData.GetBytes() }, types.CallbackData{ CallbackAddress: sender, @@ -204,19 +200,10 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { }, nil, }, - { - "failure: invalid packet data", - func() { - packetData = []byte("invalid packet data") - }, - types.CallbackData{}, - types.ErrCannotUnmarshalPacketData, - }, { "failure: packet data does not implement PacketDataProvider", func() { packetData = ibcmock.MockPacketData - packetDataUnmarshaler = ibcmock.IBCModule{} }, types.CallbackData{}, types.ErrNotPacketDataProvider, @@ -224,14 +211,13 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { { "failure: empty memo", func() { - expPacketData := transfertypes.FungibleTokenPacketData{ + packetData = transfertypes.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: sender, Receiver: receiver, Memo: "", } - packetData = expPacketData.GetBytes() }, types.CallbackData{}, types.ErrCallbackKeyNotFound, @@ -239,14 +225,13 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { { "failure: empty address", func() { - expPacketData := transfertypes.FungibleTokenPacketData{ + packetData = transfertypes.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: sender, Receiver: receiver, Memo: `{"src_callback": {"address": ""}}`, } - packetData = expPacketData.GetBytes() }, types.CallbackData{}, types.ErrCallbackAddressNotFound, @@ -254,14 +239,13 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { { "failure: space address", func() { - expPacketData := transfertypes.FungibleTokenPacketData{ + packetData = transfertypes.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: sender, Receiver: receiver, Memo: `{"src_callback": {"address": " "}}`, } - packetData = expPacketData.GetBytes() }, types.CallbackData{}, types.ErrCallbackAddressNotFound, @@ -271,13 +255,13 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { for _, tc := range testCases { tc := tc s.Run(tc.name, func() { - callbackKey = types.SourceCallbackKey + s.SetupTest() - packetDataUnmarshaler = transfer.IBCModule{} + callbackKey = types.SourceCallbackKey tc.malleate() - callbackData, err := types.GetCallbackData(packetDataUnmarshaler, packetData, ibcmock.PortID, remainingGas, uint64(1_000_000), callbackKey) + callbackData, err := types.GetCallbackData(packetData, transfertypes.PortID, remainingGas, uint64(1_000_000), callbackKey) expPass := tc.expError == nil if expPass { @@ -294,6 +278,8 @@ func (s *CallbacksTypesTestSuite) TestGetCallbackData() { } func (s *CallbacksTypesTestSuite) TestGetSourceCallbackDataTransfer() { + s.SetupTest() + sender := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() receiver := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() @@ -313,14 +299,32 @@ func (s *CallbacksTypesTestSuite) TestGetSourceCallbackDataTransfer() { CommitGasLimit: 1_000_000, } - packetUnmarshaler := transfer.IBCModule{} + s.path.EndpointA.ChannelConfig.Version = transfertypes.V1 + s.path.EndpointA.ChannelConfig.PortID = transfertypes.ModuleName + s.path.EndpointB.ChannelConfig.Version = transfertypes.V1 + s.path.EndpointB.ChannelConfig.PortID = transfertypes.ModuleName + + transferStack, ok := s.chainA.App.GetIBCKeeper().PortKeeper.Route(transfertypes.ModuleName) + s.Require().True(ok) + + packetUnmarshaler, ok := transferStack.(types.CallbacksCompatibleModule) + s.Require().True(ok) + + s.path.Setup() - callbackData, err := types.GetSourceCallbackData(packetUnmarshaler, packetDataBytes, ibcmock.PortID, 2_000_000, 1_000_000) + // Set up gas meter for context. + gasMeter := storetypes.NewGasMeter(2_000_000) + ctx := s.chainA.GetContext().WithGasMeter(gasMeter) + + packet := channeltypes.NewPacket(packetDataBytes, 0, transfertypes.PortID, s.path.EndpointA.ChannelID, transfertypes.PortID, s.path.EndpointB.ChannelID, clienttypes.ZeroHeight(), 0) + callbackData, err := types.GetSourceCallbackData(ctx, packetUnmarshaler, packet, 1_000_000) s.Require().NoError(err) s.Require().Equal(expCallbackData, callbackData) } func (s *CallbacksTypesTestSuite) TestGetDestCallbackDataTransfer() { + s.SetupTest() + sender := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() receiver := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() @@ -340,9 +344,23 @@ func (s *CallbacksTypesTestSuite) TestGetDestCallbackDataTransfer() { CommitGasLimit: 1_000_000, } - packetUnmarshaler := transfer.IBCModule{} + s.path.EndpointA.ChannelConfig.Version = transfertypes.V1 + s.path.EndpointA.ChannelConfig.PortID = transfertypes.ModuleName + s.path.EndpointB.ChannelConfig.Version = transfertypes.V1 + s.path.EndpointB.ChannelConfig.PortID = transfertypes.ModuleName + + transferStack, ok := s.chainA.App.GetIBCKeeper().PortKeeper.Route(transfertypes.ModuleName) + s.Require().True(ok) + + packetUnmarshaler, ok := transferStack.(types.CallbacksCompatibleModule) + s.Require().True(ok) + + s.path.Setup() - callbackData, err := types.GetDestCallbackData(packetUnmarshaler, packetDataBytes, ibcmock.PortID, 2_000_000, 1_000_000) + gasMeter := storetypes.NewGasMeter(2_000_000) + ctx := s.chainA.GetContext().WithGasMeter(gasMeter) + packet := channeltypes.NewPacket(packetDataBytes, 0, transfertypes.PortID, s.path.EndpointA.ChannelID, transfertypes.PortID, s.path.EndpointB.ChannelID, clienttypes.ZeroHeight(), 0) + callbackData, err := types.GetDestCallbackData(ctx, packetUnmarshaler, packet, 1_000_000) s.Require().NoError(err) s.Require().Equal(expCallbackData, callbackData) } diff --git a/modules/apps/callbacks/types/export_test.go b/modules/apps/callbacks/types/export_test.go index 5ba59bed4ca..4cc85624827 100644 --- a/modules/apps/callbacks/types/export_test.go +++ b/modules/apps/callbacks/types/export_test.go @@ -1,20 +1,15 @@ package types -import ( - porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types" -) - /* This file is to allow for unexported functions to be accessible to the testing package. */ // GetCallbackData is a wrapper around getCallbackData to allow the function to be directly called in tests. func GetCallbackData( - packetDataUnmarshaler porttypes.PacketDataUnmarshaler, - packetData []byte, srcPortID string, remainingGas, + packetData interface{}, srcPortID string, remainingGas, maxGas uint64, callbackKey string, ) (CallbackData, error) { - return getCallbackData(packetDataUnmarshaler, packetData, srcPortID, remainingGas, maxGas, callbackKey) + return getCallbackData(packetData, srcPortID, remainingGas, maxGas, callbackKey) } // GetCallbackAddress is a wrapper around getCallbackAddress to allow the function to be directly called in tests. diff --git a/modules/apps/callbacks/types/types_test.go b/modules/apps/callbacks/types/types_test.go index 7bb61f95162..5aab45242d2 100644 --- a/modules/apps/callbacks/types/types_test.go +++ b/modules/apps/callbacks/types/types_test.go @@ -14,13 +14,17 @@ type CallbacksTypesTestSuite struct { coord *ibctesting.Coordinator - chain *ibctesting.TestChain + chainA, chainB *ibctesting.TestChain + + path *ibctesting.Path } -// SetupTest creates a coordinator with 1 test chain. -func (s *CallbacksTypesTestSuite) SetupSuite() { - s.coord = ibctesting.NewCoordinator(s.T(), 1) - s.chain = s.coord.GetChain(ibctesting.GetChainID(1)) +// SetupTest creates a coordinator with 2 test chains. +func (s *CallbacksTypesTestSuite) SetupTest() { + s.coord = ibctesting.NewCoordinator(s.T(), 2) + s.chainA = s.coord.GetChain(ibctesting.GetChainID(1)) + s.chainB = s.coord.GetChain(ibctesting.GetChainID(2)) + s.path = ibctesting.NewPath(s.chainA, s.chainB) } func TestCallbacksTypesTestSuite(t *testing.T) { diff --git a/modules/apps/transfer/client/cli/tx.go b/modules/apps/transfer/client/cli/tx.go index 497e56370d5..b0ca95fe7fe 100644 --- a/modules/apps/transfer/client/cli/tx.go +++ b/modules/apps/transfer/client/cli/tx.go @@ -34,12 +34,14 @@ var defaultRelativePacketTimeoutTimestamp = uint64((time.Duration(10) * time.Min // NewTransferTxCmd returns the command to create a NewMsgTransfer transaction func NewTransferTxCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "transfer [src-port] [src-channel] [receiver] [amount]", - Short: "Transfer a fungible token through IBC", - Long: strings.TrimSpace(`Transfer a fungible token through IBC. Timeouts can be specified as absolute using the {absolute-timeouts} flag. -Timeout height can be set by passing in the height string in the form {revision}-{height} using the {packet-timeout-height} flag. Note, relative timeout height is not supported. -Relative timeout timestamp is added to the value of the user's local system clock time using the {packet-timeout-timestamp} flag. If no timeout value is set then a default relative timeout value of 10 minutes is used.`), - Example: fmt.Sprintf("%s tx ibc-transfer transfer [src-port] [src-channel] [receiver] [amount]", version.AppName), + Use: "transfer [src-port] [src-channel] [receiver] [coins]", + Short: "Transfer one or more fungible tokens through IBC", + Long: strings.TrimSpace(`Transfer one or more fungible tokens through IBC. Multiple tokens can be transferred in a single +packet if the coins list is a comma-separated string (e.g. 100uatom,100uosmo). Timeouts can be specified as absolute using the {absolute-timeouts} flag. +Timeout height can be set by passing in the height string in the form {revision}-{height} using the {packet-timeout-height} flag. +Note, relative timeout height is not supported. Relative timeout timestamp is added to the value of the user's local system clock time +using the {packet-timeout-timestamp} flag. If no timeout value is set then a default relative timeout value of 10 minutes is used.`), + Example: fmt.Sprintf("%s tx ibc-transfer transfer [src-port] [src-channel] [receiver] [coins]", version.AppName), Args: cobra.ExactArgs(4), RunE: func(cmd *cobra.Command, args []string) error { clientCtx, err := client.GetClientTxContext(cmd) @@ -51,14 +53,16 @@ Relative timeout timestamp is added to the value of the user's local system cloc srcChannel := args[1] receiver := args[2] - coin, err := sdk.ParseCoinNormalized(args[3]) + coins, err := sdk.ParseCoinsNormalized(args[3]) if err != nil { return err } - if !strings.HasPrefix(coin.Denom, "ibc/") { - denomTrace := types.ParseDenomTrace(coin.Denom) - coin.Denom = denomTrace.IBCDenom() + for i, coin := range coins { + if !strings.HasPrefix(coin.Denom, "ibc/") { + denomTrace := types.ParseDenomTrace(coin.Denom) + coins[i].Denom = denomTrace.IBCDenom() + } } timeoutHeightStr, err := cmd.Flags().GetString(flagPacketTimeoutHeight) @@ -107,7 +111,7 @@ Relative timeout timestamp is added to the value of the user's local system cloc } msg := types.NewMsgTransfer( - srcPort, srcChannel, coin, sender, receiver, timeoutHeight, timeoutTimestamp, memo, + srcPort, srcChannel, coins, sender, receiver, timeoutHeight, timeoutTimestamp, memo, ) return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg) }, diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go index 7e312b7b869..f8435946c47 100644 --- a/modules/apps/transfer/ibc_module.go +++ b/modules/apps/transfer/ibc_module.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "math" + "slices" "strings" errorsmod "cosmossdk.io/errors" @@ -11,6 +12,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types" + convertinternal "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/convert" "github.com/cosmos/ibc-go/v8/modules/apps/transfer/keeper" "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" @@ -85,12 +87,13 @@ func (im IBCModule) OnChanOpenInit( return "", err } + // default to latest supported version if strings.TrimSpace(version) == "" { - version = types.Version + version = types.V2 } - if version != types.Version { - return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, version) + if !slices.Contains(types.SupportedVersions, version) { + return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected one of %s, got %s", types.SupportedVersions, version) } // Claim channel capability passed back by IBC module @@ -121,13 +124,11 @@ func (im IBCModule) OnChanOpenTry( return "", err } - if counterpartyVersion != types.Version { - // Propose the current version - im.keeper.Logger(ctx).Debug("invalid counterparty version, proposing current app version", "counterpartyVersion", counterpartyVersion, "version", types.Version) - return types.Version, nil + if !slices.Contains(types.SupportedVersions, counterpartyVersion) { + return types.V2, nil } - return types.Version, nil + return counterpartyVersion, nil } // OnChanOpenAck implements the IBCModule interface @@ -138,9 +139,10 @@ func (IBCModule) OnChanOpenAck( _ string, counterpartyVersion string, ) error { - if counterpartyVersion != types.Version { - return errorsmod.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: expected %s, got %s", types.Version, counterpartyVersion) + if !slices.Contains(types.SupportedVersions, counterpartyVersion) { + return errorsmod.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: expected one of %s, got %s", types.SupportedVersions, counterpartyVersion) } + return nil } @@ -172,6 +174,37 @@ func (IBCModule) OnChanCloseConfirm( return nil } +// unmarshalPacketDataBytesToICS20V2 attempts to unmarshal the provided packet data bytes into a FungibleTokenPacketDataV2. +// The version of ics20 should be provided and should be either ics20-1 or ics20-2. +func (IBCModule) unmarshalPacketDataBytesToICS20V2(bz []byte, ics20Version string) (types.FungibleTokenPacketDataV2, error) { + switch ics20Version { + case types.V1: + var datav1 types.FungibleTokenPacketData + if err := json.Unmarshal(bz, &datav1); err != nil { + return types.FungibleTokenPacketDataV2{}, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS20-V1 transfer packet data: %s", err.Error()) + } + + if err := datav1.ValidateBasic(); err != nil { + return types.FungibleTokenPacketDataV2{}, err + } + + return convertinternal.PacketDataV1ToV2(datav1), nil + case types.V2: + var datav2 types.FungibleTokenPacketDataV2 + if err := json.Unmarshal(bz, &datav2); err != nil { + return types.FungibleTokenPacketDataV2{}, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS20-V2 transfer packet data: %s", err.Error()) + } + + if err := datav2.ValidateBasic(); err != nil { + return types.FungibleTokenPacketDataV2{}, err + } + + return datav2, nil + default: + return types.FungibleTokenPacketDataV2{}, errorsmod.Wrap(types.ErrInvalidVersion, ics20Version) + } +} + // OnRecvPacket implements the IBCModule interface. A successful acknowledgement // is returned if the packet data is successfully decoded and the receive application // logic returns without error. @@ -182,10 +215,14 @@ func (im IBCModule) OnRecvPacket( ) ibcexported.Acknowledgement { ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)}) - var data types.FungibleTokenPacketData - var ackErr error - if err := json.Unmarshal(packet.GetData(), &data); err != nil { - ackErr = errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS-20 transfer packet data") + ics20Version, found := im.keeper.GetICS4Wrapper().GetAppVersion(ctx, packet.DestinationPort, packet.DestinationChannel) + if !found { + return channeltypes.NewErrorAcknowledgement(errorsmod.Wrapf(ibcerrors.ErrNotFound, "app version not found for port %s and channel %s", packet.DestinationPort, packet.DestinationChannel)) + } + + data, ackErr := im.unmarshalPacketDataBytesToICS20V2(packet.GetData(), ics20Version) + if ackErr != nil { + ackErr = errorsmod.Wrapf(ibcerrors.ErrInvalidType, ackErr.Error()) im.keeper.Logger(ctx).Error(fmt.Sprintf("%s sequence %d", ackErr.Error(), packet.Sequence)) ack = channeltypes.NewErrorAcknowledgement(ackErr) } @@ -204,11 +241,9 @@ func (im IBCModule) OnRecvPacket( } eventAttributes := []sdk.Attribute{ - sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), - sdk.NewAttribute(sdk.AttributeKeySender, data.Sender), + sdk.NewAttribute(types.AttributeKeySender, data.Sender), sdk.NewAttribute(types.AttributeKeyReceiver, data.Receiver), - sdk.NewAttribute(types.AttributeKeyDenom, data.Denom), - sdk.NewAttribute(types.AttributeKeyAmount, data.Amount), + sdk.NewAttribute(types.AttributeKeyTokens, types.Tokens(data.Tokens).String()), sdk.NewAttribute(types.AttributeKeyMemo, data.Memo), sdk.NewAttribute(types.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack.Success())), } @@ -217,12 +252,16 @@ func (im IBCModule) OnRecvPacket( eventAttributes = append(eventAttributes, sdk.NewAttribute(types.AttributeKeyAckError, ackErr.Error())) } - ctx.EventManager().EmitEvent( + ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypePacket, eventAttributes..., ), - ) + sdk.NewEvent( + sdk.EventTypeMessage, + sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), + ), + }) // NOTE: acknowledgement will be written synchronously during IBC handler execution. return ack @@ -239,27 +278,35 @@ func (im IBCModule) OnAcknowledgementPacket( if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil { return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err) } - var data types.FungibleTokenPacketData - if err := json.Unmarshal(packet.GetData(), &data); err != nil { - return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error()) + + ics20Version, found := im.keeper.GetICS4Wrapper().GetAppVersion(ctx, packet.SourcePort, packet.SourceChannel) + if !found { + return errorsmod.Wrapf(ibcerrors.ErrNotFound, "app version not found for port %s and channel %s", packet.SourcePort, packet.SourceChannel) + } + + data, err := im.unmarshalPacketDataBytesToICS20V2(packet.GetData(), ics20Version) + if err != nil { + return err } if err := im.keeper.OnAcknowledgementPacket(ctx, packet, data, ack); err != nil { return err } - ctx.EventManager().EmitEvent( + ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypePacket, - sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), sdk.NewAttribute(sdk.AttributeKeySender, data.Sender), sdk.NewAttribute(types.AttributeKeyReceiver, data.Receiver), - sdk.NewAttribute(types.AttributeKeyDenom, data.Denom), - sdk.NewAttribute(types.AttributeKeyAmount, data.Amount), + sdk.NewAttribute(types.AttributeKeyTokens, types.Tokens(data.Tokens).String()), sdk.NewAttribute(types.AttributeKeyMemo, data.Memo), sdk.NewAttribute(types.AttributeKeyAck, ack.String()), ), - ) + sdk.NewEvent( + sdk.EventTypeMessage, + sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), + ), + }) switch resp := ack.Response.(type) { case *channeltypes.Acknowledgement_Result: @@ -287,25 +334,33 @@ func (im IBCModule) OnTimeoutPacket( packet channeltypes.Packet, relayer sdk.AccAddress, ) error { - var data types.FungibleTokenPacketData - if err := json.Unmarshal(packet.GetData(), &data); err != nil { - return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error()) + ics20Version, found := im.keeper.GetICS4Wrapper().GetAppVersion(ctx, packet.SourcePort, packet.SourceChannel) + if !found { + return errorsmod.Wrapf(ibcerrors.ErrNotFound, "app version not found for port %s and channel %s", packet.SourcePort, packet.SourceChannel) + } + + data, err := im.unmarshalPacketDataBytesToICS20V2(packet.GetData(), ics20Version) + if err != nil { + return err } + // refund tokens if err := im.keeper.OnTimeoutPacket(ctx, packet, data); err != nil { return err } - ctx.EventManager().EmitEvent( + ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypeTimeout, - sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), - sdk.NewAttribute(types.AttributeKeyRefundReceiver, data.Sender), - sdk.NewAttribute(types.AttributeKeyRefundDenom, data.Denom), - sdk.NewAttribute(types.AttributeKeyRefundAmount, data.Amount), + sdk.NewAttribute(types.AttributeKeyReceiver, data.Sender), + sdk.NewAttribute(types.AttributeKeyRefundTokens, types.Tokens(data.Tokens).String()), sdk.NewAttribute(types.AttributeKeyMemo, data.Memo), ), - ) + sdk.NewEvent( + sdk.EventTypeMessage, + sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), + ), + }) return nil } @@ -316,8 +371,8 @@ func (im IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, return "", err } - if proposedVersion != types.Version { - return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, proposedVersion) + if !slices.Contains(types.SupportedVersions, proposedVersion) { + return "", errorsmod.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: expected one of %s, got %s", types.SupportedVersions, proposedVersion) } return proposedVersion, nil @@ -329,8 +384,8 @@ func (im IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, return "", err } - if counterpartyVersion != types.Version { - return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, counterpartyVersion) + if !slices.Contains(types.SupportedVersions, counterpartyVersion) { + return types.V2, nil } return counterpartyVersion, nil @@ -338,8 +393,8 @@ func (im IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, // OnChanUpgradeAck implements the IBCModule interface func (IBCModule) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error { - if counterpartyVersion != types.Version { - return errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, counterpartyVersion) + if !slices.Contains(types.SupportedVersions, counterpartyVersion) { + return errorsmod.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: expected one of %s, got %s", types.SupportedVersions, counterpartyVersion) } return nil @@ -352,11 +407,16 @@ func (IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, pr // UnmarshalPacketData attempts to unmarshal the provided packet data bytes // into a FungibleTokenPacketData. This function implements the optional // PacketDataUnmarshaler interface required for ADR 008 support. -func (IBCModule) UnmarshalPacketData(bz []byte) (interface{}, error) { - var packetData types.FungibleTokenPacketData - if err := json.Unmarshal(bz, &packetData); err != nil { +func (im IBCModule) UnmarshalPacketData(ctx sdk.Context, portID, channelID string, bz []byte) (interface{}, error) { + ics20Version, found := im.keeper.GetICS4Wrapper().GetAppVersion(ctx, portID, channelID) + if !found { + return nil, errorsmod.Wrapf(ibcerrors.ErrNotFound, "app version not found for port %s and channel %s", portID, channelID) + } + + ftpd, err := im.unmarshalPacketDataBytesToICS20V2(bz, ics20Version) + if err != nil { return nil, err } - return packetData, nil + return ftpd, nil } diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go index 4017134a999..71f261480ed 100644 --- a/modules/apps/transfer/ibc_module_test.go +++ b/modules/apps/transfer/ibc_module_test.go @@ -26,50 +26,56 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() { ) testCases := []struct { - name string - malleate func() - expError error + name string + malleate func() + expError error + expVersion string }{ { - "success", func() {}, nil, + "success", func() {}, nil, types.V2, }, { // connection hops is not used in the transfer application callback, // it is already validated in the core OnChanUpgradeInit. "success: invalid connection hops", func() { path.EndpointA.ConnectionID = "invalid-connection-id" - }, nil, + }, nil, types.V2, }, { - "empty version string", func() { + "success: empty version string", func() { channel.Version = "" - }, nil, + }, nil, types.V2, + }, + { + "success: ics20-1 legacy", func() { + channel.Version = types.V1 + }, nil, types.V1, }, { "max channels reached", func() { path.EndpointA.ChannelID = channeltypes.FormatChannelIdentifier(math.MaxUint32 + 1) - }, types.ErrMaxTransferChannels, + }, types.ErrMaxTransferChannels, "", }, { "invalid order - ORDERED", func() { channel.Ordering = channeltypes.ORDERED - }, channeltypes.ErrInvalidChannelOrdering, + }, channeltypes.ErrInvalidChannelOrdering, "", }, { "invalid port ID", func() { path.EndpointA.ChannelConfig.PortID = ibctesting.MockPort - }, porttypes.ErrInvalidPort, + }, porttypes.ErrInvalidPort, "", }, { "invalid version", func() { channel.Version = "version" //nolint:goconst - }, types.ErrInvalidVersion, + }, types.ErrInvalidVersion, "", }, { "capability already claimed", func() { err := suite.chainA.GetSimApp().ScopedTransferKeeper.ClaimCapability(suite.chainA.GetContext(), chanCap, host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) suite.Require().NoError(err) - }, capabilitytypes.ErrOwnerClaimed, + }, capabilitytypes.ErrOwnerClaimed, "", }, } @@ -88,7 +94,7 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() { Ordering: channeltypes.UNORDERED, Counterparty: counterparty, ConnectionHops: []string{path.EndpointA.ConnectionID}, - Version: types.Version, + Version: types.V2, } var err error @@ -105,7 +111,7 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() { expPass := tc.expError == nil if expPass { suite.Require().NoError(err) - suite.Require().Equal(types.Version, version) + suite.Require().Equal(tc.expVersion, version) } else { suite.Require().Error(err) suite.Require().Contains(err.Error(), tc.expError.Error()) @@ -124,39 +130,45 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() { ) testCases := []struct { - name string - malleate func() - expError error + name string + malleate func() + expError error + expVersion string }{ { - "success", func() {}, nil, + "success", func() {}, nil, types.V2, + }, + { + "success: counterparty version is legacy ics20-1", func() { + counterpartyVersion = types.V1 + }, nil, types.V1, }, { - "success: invalid counterparty version proposes new version", func() { + "success: invalid counterparty version, we propose new version", func() { // transfer module will propose the default version counterpartyVersion = "version" - }, nil, + }, nil, types.V2, }, { "failure: max channels reached", func() { path.EndpointA.ChannelID = channeltypes.FormatChannelIdentifier(math.MaxUint32 + 1) - }, types.ErrMaxTransferChannels, + }, types.ErrMaxTransferChannels, "", }, { "failure: capability already claimed", func() { err := suite.chainA.GetSimApp().ScopedTransferKeeper.ClaimCapability(suite.chainA.GetContext(), chanCap, host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) suite.Require().NoError(err) - }, capabilitytypes.ErrOwnerClaimed, + }, capabilitytypes.ErrOwnerClaimed, "", }, { "failure: invalid order - ORDERED", func() { channel.Ordering = channeltypes.ORDERED - }, channeltypes.ErrInvalidChannelOrdering, + }, channeltypes.ErrInvalidChannelOrdering, "", }, { "failure: invalid port ID", func() { path.EndpointA.ChannelConfig.PortID = ibctesting.MockPort - }, porttypes.ErrInvalidPort, + }, porttypes.ErrInvalidPort, "", }, } @@ -176,9 +188,9 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() { Ordering: channeltypes.UNORDERED, Counterparty: counterparty, ConnectionHops: []string{path.EndpointA.ConnectionID}, - Version: types.Version, + Version: types.V2, } - counterpartyVersion = types.Version + counterpartyVersion = types.V2 module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort) suite.Require().NoError(err) @@ -197,7 +209,7 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() { expPass := tc.expError == nil if expPass { suite.Require().NoError(err) - suite.Require().Equal(types.Version, version) + suite.Require().Equal(tc.expVersion, version) } else { suite.Require().Error(err) suite.Require().Contains(err.Error(), tc.expError.Error()) @@ -233,7 +245,7 @@ func (suite *TransferTestSuite) TestOnChanOpenAck() { path := ibctesting.NewTransferPath(suite.chainA, suite.chainB) path.SetupConnections() path.EndpointA.ChannelID = ibctesting.FirstChannelID - counterpartyVersion = types.Version + counterpartyVersion = types.V2 module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort) suite.Require().NoError(err) @@ -344,18 +356,18 @@ func (suite *TransferTestSuite) TestOnChanUpgradeTry() { nil, }, { - "invalid upgrade ordering", + "success: invalid upgrade version from counterparty, we use our proposed version", func() { - counterpartyUpgrade.Fields.Ordering = channeltypes.ORDERED + counterpartyUpgrade.Fields.Version = ibctesting.InvalidID }, - channeltypes.ErrInvalidChannelOrdering, + nil, }, { - "invalid upgrade version", + "invalid upgrade ordering", func() { - counterpartyUpgrade.Fields.Version = ibctesting.InvalidID + counterpartyUpgrade.Fields.Ordering = channeltypes.ORDERED }, - types.ErrInvalidVersion, + channeltypes.ErrInvalidChannelOrdering, }, } @@ -398,7 +410,7 @@ func (suite *TransferTestSuite) TestOnChanUpgradeTry() { expPass := tc.expError == nil if expPass { suite.Require().NoError(err) - suite.Require().Equal(types.Version, version) + suite.Require().Equal(types.V2, version) } else { suite.Require().Error(err) suite.Require().Contains(err.Error(), tc.expError.Error()) @@ -479,8 +491,9 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() { sender = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() receiver = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() - data []byte - expPacketData types.FungibleTokenPacketData + path *ibctesting.Path + data []byte + initialPacketData interface{} ) testCases := []struct { @@ -489,53 +502,157 @@ func (suite *TransferTestSuite) TestPacketDataUnmarshalerInterface() { expError error }{ { - "success: valid packet data with memo", + "success: valid packet data single denom -> multidenom conversion with memo", func() { - expPacketData = types.FungibleTokenPacketData{ + path.EndpointA.ChannelConfig.Version = types.V1 + initialPacketData = types.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: sender, Receiver: receiver, Memo: "some memo", } - data = expPacketData.GetBytes() + + data = initialPacketData.(types.FungibleTokenPacketData).GetBytes() }, nil, }, { - "success: valid packet data without memo", + "success: valid packet data single denom -> multidenom conversion without memo", func() { - expPacketData = types.FungibleTokenPacketData{ + path.EndpointA.ChannelConfig.Version = types.V1 + initialPacketData = types.FungibleTokenPacketData{ Denom: ibctesting.TestCoin.Denom, Amount: ibctesting.TestCoin.Amount.String(), Sender: sender, Receiver: receiver, Memo: "", } - data = expPacketData.GetBytes() + + data = initialPacketData.(types.FungibleTokenPacketData).GetBytes() + }, + nil, + }, + { + "success: valid packet data single denom with trace -> multidenom conversion with trace", + func() { + path.EndpointA.ChannelConfig.Version = types.V1 + initialPacketData = types.FungibleTokenPacketData{ + Denom: "transfer/channel-0/atom", + Amount: ibctesting.TestCoin.Amount.String(), + Sender: sender, + Receiver: receiver, + Memo: "", + } + + data = initialPacketData.(types.FungibleTokenPacketData).GetBytes() + }, + nil, + }, + { + "success: valid packet data multidenom with memo", + func() { + initialPacketData = types.FungibleTokenPacketDataV2{ + Tokens: []types.Token{ + { + Denom: "atom", + Amount: ibctesting.TestCoin.Amount.String(), + Trace: []string{"transfer/channel-0"}, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: "some memo", + } + + data = initialPacketData.(types.FungibleTokenPacketDataV2).GetBytes() + }, + nil, + }, + { + "success: valid packet data multidenom nil trace", + func() { + path.EndpointA.ChannelConfig.Version = types.V2 + initialPacketData = types.FungibleTokenPacketDataV2{ + Tokens: []types.Token{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + Trace: nil, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: "", + } + + data = initialPacketData.(types.FungibleTokenPacketDataV2).GetBytes() }, nil, }, + { + "failure: invalid token trace", + func() { + path.EndpointA.ChannelConfig.Version = types.V2 + initialPacketData = types.FungibleTokenPacketDataV2{ + Tokens: []types.Token{ + { + Denom: ibctesting.TestCoin.Denom, + Amount: ibctesting.TestCoin.Amount.String(), + Trace: []string{""}, + }, + }, + Sender: sender, + Receiver: receiver, + Memo: "", + } + + data = initialPacketData.(types.FungibleTokenPacketDataV2).GetBytes() + }, + errors.New("trace info must come in pairs of port and channel identifiers"), + }, { "failure: invalid packet data", func() { data = []byte("invalid packet data") }, - errors.New("invalid character 'i' looking for beginning of value"), + errors.New("cannot unmarshal ICS20-V2 transfer packet data"), }, } for _, tc := range testCases { tc := tc suite.Run(tc.name, func() { + path = ibctesting.NewTransferPath(suite.chainA, suite.chainB) + tc.malleate() - packetData, err := transfer.IBCModule{}.UnmarshalPacketData(data) + path.Setup() + + transferStack, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(types.ModuleName) + suite.Require().True(ok) + + unmarshalerStack, ok := transferStack.(porttypes.PacketDataUnmarshaler) + suite.Require().True(ok) + + packetData, err := unmarshalerStack.UnmarshalPacketData(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, data) expPass := tc.expError == nil if expPass { suite.Require().NoError(err) - suite.Require().Equal(expPacketData, packetData) + + v2PacketData, ok := packetData.(types.FungibleTokenPacketDataV2) + suite.Require().True(ok) + + if v1PacketData, ok := initialPacketData.(types.FungibleTokenPacketData); ok { + // Note: testing of the denom trace parsing/conversion should be done as part of testing internal conversion functions + suite.Require().Equal(v1PacketData.Amount, v2PacketData.Tokens[0].Amount) + suite.Require().Equal(v1PacketData.Sender, v2PacketData.Sender) + suite.Require().Equal(v1PacketData.Receiver, v2PacketData.Receiver) + suite.Require().Equal(v1PacketData.Memo, v2PacketData.Memo) + } else { + suite.Require().Equal(initialPacketData.(types.FungibleTokenPacketDataV2), v2PacketData) + } } else { suite.Require().Error(err) suite.Require().Contains(err.Error(), tc.expError.Error()) diff --git a/modules/apps/transfer/internal/convert/convert.go b/modules/apps/transfer/internal/convert/convert.go new file mode 100644 index 00000000000..686ae6119b8 --- /dev/null +++ b/modules/apps/transfer/internal/convert/convert.go @@ -0,0 +1,54 @@ +package convert + +import ( + "strings" + + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" +) + +// PacketDataV1ToV2 converts a v1 packet data to a v2 packet data. +func PacketDataV1ToV2(packetData types.FungibleTokenPacketData) types.FungibleTokenPacketDataV2 { + if err := packetData.ValidateBasic(); err != nil { + panic(err) + } + + v2Denom, trace := ExtractDenomAndTraceFromV1Denom(packetData.Denom) + return types.FungibleTokenPacketDataV2{ + Tokens: []types.Token{ + { + Denom: v2Denom, + Amount: packetData.Amount, + Trace: trace, + }, + }, + Sender: packetData.Sender, + Receiver: packetData.Receiver, + Memo: packetData.Memo, + } +} + +// extractDenomAndTraceFromV1Denom extracts the base denom and remaining trace from a v1 IBC denom. +func ExtractDenomAndTraceFromV1Denom(v1Denom string) (string, []string) { + v1DenomTrace := types.ParseDenomTrace(v1Denom) + + // if the path string is empty, then the base denom is the full native denom. + if v1DenomTrace.Path == "" { + return v1DenomTrace.BaseDenom, nil + } + + splitPath := strings.Split(v1DenomTrace.Path, "/") + + // this condition should never be reached. + if len(splitPath)%2 != 0 { + panic("pathSlice length is not even") + } + + // the path slices consists of entries of ports and channel ids separately, + // we need to combine them to form the trace. + var trace []string + for i := 0; i < len(splitPath); i += 2 { + trace = append(trace, strings.Join(splitPath[i:i+2], "/")) + } + + return v1DenomTrace.BaseDenom, trace +} diff --git a/modules/apps/transfer/internal/convert/convert_test.go b/modules/apps/transfer/internal/convert/convert_test.go new file mode 100644 index 00000000000..10aecb3c33d --- /dev/null +++ b/modules/apps/transfer/internal/convert/convert_test.go @@ -0,0 +1,135 @@ +package convert + +import ( + "testing" + + "github.com/stretchr/testify/require" + + errorsmod "cosmossdk.io/errors" + + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" +) + +func TestConvertPacketV1ToPacketV2(t *testing.T) { + const ( + sender = "sender" + receiver = "receiver" + ) + + testCases := []struct { + name string + v1Data types.FungibleTokenPacketData + v2Data types.FungibleTokenPacketDataV2 + expPanic error + }{ + { + "success", + types.NewFungibleTokenPacketData("transfer/channel-0/atom", "1000", sender, receiver, ""), + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: "atom", + Amount: "1000", + Trace: []string{"transfer/channel-0"}, + }, + }, sender, receiver, ""), + nil, + }, + { + "success with empty trace", + types.NewFungibleTokenPacketData("atom", "1000", sender, receiver, ""), + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: "atom", + Amount: "1000", + Trace: nil, + }, + }, sender, receiver, ""), + nil, + }, + { + "success: base denom with '/'", + types.NewFungibleTokenPacketData("transfer/channel-0/atom/withslash", "1000", sender, receiver, ""), + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: "atom/withslash", + Amount: "1000", + Trace: []string{"transfer/channel-0"}, + }, + }, sender, receiver, ""), + nil, + }, + { + "success: base denom with '/' at the end", + types.NewFungibleTokenPacketData("transfer/channel-0/atom/", "1000", sender, receiver, ""), + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: "atom/", + Amount: "1000", + Trace: []string{"transfer/channel-0"}, + }, + }, sender, receiver, ""), + nil, + }, + { + "success: longer trace base denom with '/'", + types.NewFungibleTokenPacketData("transfer/channel-0/transfer/channel-1/atom/pool", "1000", sender, receiver, ""), + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, sender, receiver, ""), + nil, + }, + { + "success: longer trace with non transfer port", + types.NewFungibleTokenPacketData("transfer/channel-0/transfer/channel-1/transfer-custom/channel-2/atom", "1000", sender, receiver, ""), + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: "atom", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, + }, + }, sender, receiver, ""), + nil, + }, + { + "success: base denom with slash, trace with non transfer port", + types.NewFungibleTokenPacketData("transfer/channel-0/transfer/channel-1/transfer-custom/channel-2/atom/pool", "1000", sender, receiver, ""), + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: "atom/pool", + Amount: "1000", + Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, + }, + }, sender, receiver, ""), + nil, + }, + { + "failure: panics with empty denom", + types.NewFungibleTokenPacketData("", "1000", sender, receiver, ""), + types.FungibleTokenPacketDataV2{}, + errorsmod.Wrap(types.ErrInvalidDenomForTransfer, "base denomination cannot be blank"), + }, + } + + for _, tc := range testCases { + expPass := tc.expPanic == nil + if expPass { + actualV2Data := PacketDataV1ToV2(tc.v1Data) + require.Equal(t, tc.v2Data, actualV2Data, "test case: %s", tc.name) + } else { + require.PanicsWithError(t, tc.expPanic.Error(), func() { + PacketDataV1ToV2(tc.v1Data) + }, "test case: %s", tc.name) + } + } +} diff --git a/modules/apps/transfer/keeper/invariants_test.go b/modules/apps/transfer/keeper/invariants_test.go index ef1b6ae90ca..e150f9ef7c9 100644 --- a/modules/apps/transfer/keeper/invariants_test.go +++ b/modules/apps/transfer/keeper/invariants_test.go @@ -47,7 +47,7 @@ func (suite *KeeperTestSuite) TestTotalEscrowPerDenomInvariant() { msg := types.NewMsgTransfer( path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - coin, + sdk.NewCoins(coin), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.GetTimeoutHeight(), 0, "", diff --git a/modules/apps/transfer/keeper/mbt_relay_test.go b/modules/apps/transfer/keeper/mbt_relay_test.go index fcb466dd468..d306643079a 100644 --- a/modules/apps/transfer/keeper/mbt_relay_test.go +++ b/modules/apps/transfer/keeper/mbt_relay_test.go @@ -19,6 +19,7 @@ import ( "github.com/cometbft/cometbft/crypto" + convertinternal "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/convert" "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" @@ -65,7 +66,7 @@ type FungibleTokenPacket struct { SourcePort string DestChannel string DestPort string - Data types.FungibleTokenPacketData + Data types.FungibleTokenPacketDataV2 } type OnRecvPacketTestCase = struct { @@ -108,7 +109,7 @@ func AddressFromTla(addr []string) string { s = addr[2] } else if len(addr[2]) == 0 { // escrow address: ics20-1\x00port/channel - s = fmt.Sprintf("%s\x00%s/%s", types.Version, addr[0], addr[1]) + s = fmt.Sprintf("%s\x00%s/%s", types.V1, addr[0], addr[1]) } else { panic(errors.New("failed to convert from TLA+ address: neither simple nor escrow address")) } @@ -143,17 +144,24 @@ func BalancesFromTla(tla []TlaBalance) []Balance { } func FungibleTokenPacketFromTla(packet TlaFungibleTokenPacket) FungibleTokenPacket { + denom, trace := convertinternal.ExtractDenomAndTraceFromV1Denom(DenomFromTla(packet.Data.Denom)) return FungibleTokenPacket{ SourceChannel: packet.SourceChannel, SourcePort: packet.SourcePort, DestChannel: packet.DestChannel, DestPort: packet.DestPort, - Data: types.NewFungibleTokenPacketData( - DenomFromTla(packet.Data.Denom), - packet.Data.Amount, + Data: types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: packet.Data.Amount, + Trace: trace, + }, + }, AddressFromString(packet.Data.Sender), AddressFromString(packet.Data.Receiver), - ""), + "", + ), } } @@ -310,7 +318,7 @@ func (suite *KeeperTestSuite) TestModelBasedRelay() { for i, tlaTc := range tlaTestCases { tc := OnRecvPacketTestCaseFromTla(tlaTc) registerDenomFn := func() { - denomTrace := types.ParseDenomTrace(tc.packet.Data.Denom) + denomTrace := types.ParseDenomTrace(tc.packet.Data.Tokens[0].GetFullDenomPath()) traceHash := denomTrace.Hash() if !suite.chainB.GetSimApp().TransferKeeper.HasDenomTrace(suite.chainB.GetContext(), traceHash) { suite.chainB.GetSimApp().TransferKeeper.SetDenomTrace(suite.chainB.GetContext(), denomTrace) @@ -337,18 +345,18 @@ func (suite *KeeperTestSuite) TestModelBasedRelay() { panic(errors.New("MBT failed to convert sender address")) } registerDenomFn() - denomTrace := types.ParseDenomTrace(tc.packet.Data.Denom) + denomTrace := types.ParseDenomTrace(tc.packet.Data.Tokens[0].GetFullDenomPath()) denom := denomTrace.IBCDenom() err = sdk.ValidateDenom(denom) if err == nil { - amount, ok := sdkmath.NewIntFromString(tc.packet.Data.Amount) + amount, ok := sdkmath.NewIntFromString(tc.packet.Data.Tokens[0].Amount) if !ok { panic(errors.New("MBT failed to parse amount from string")) } msg := types.NewMsgTransfer( tc.packet.SourcePort, tc.packet.SourceChannel, - sdk.NewCoin(denom, amount), + sdk.NewCoins(sdk.NewCoin(denom, amount)), sender.String(), tc.packet.Data.Receiver, suite.chainA.GetTimeoutHeight(), 0, // only use timeout height diff --git a/modules/apps/transfer/keeper/msg_server.go b/modules/apps/transfer/keeper/msg_server.go index c5171573226..f6a04ab4b81 100644 --- a/modules/apps/transfer/keeper/msg_server.go +++ b/modules/apps/transfer/keeper/msg_server.go @@ -26,8 +26,20 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types. return nil, err } - if !k.bankKeeper.IsSendEnabledCoin(ctx, msg.Token) { - return nil, errorsmod.Wrapf(types.ErrSendDisabled, "%s transfers are currently disabled", msg.Token.Denom) + coins := msg.GetCoins() + + appVersion, found := k.ics4Wrapper.GetAppVersion(ctx, msg.SourcePort, msg.SourceChannel) + if !found { + return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "application version not found for source port: %s and source channel: %s", msg.SourcePort, msg.SourceChannel) + } + + // ics20-1 only supports a single coin, so if that is the current version, we must only process a single coin. + if appVersion == types.V1 && len(coins) > 1 { + return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "cannot transfer multiple coins with ics20-1") + } + + if err := k.bankKeeper.IsSendEnabledCoins(ctx, coins...); err != nil { + return nil, errorsmod.Wrapf(types.ErrSendDisabled, err.Error()) } if k.bankKeeper.BlockedAddr(sender) { @@ -35,21 +47,18 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types. } sequence, err := k.sendTransfer( - ctx, msg.SourcePort, msg.SourceChannel, msg.Token, sender, msg.Receiver, msg.TimeoutHeight, msg.TimeoutTimestamp, + ctx, msg.SourcePort, msg.SourceChannel, coins, sender, msg.Receiver, msg.TimeoutHeight, msg.TimeoutTimestamp, msg.Memo) if err != nil { return nil, err } - k.Logger(ctx).Info("IBC fungible token transfer", "token", msg.Token.Denom, "amount", msg.Token.Amount.String(), "sender", msg.Sender, "receiver", msg.Receiver) - ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypeTransfer, - sdk.NewAttribute(sdk.AttributeKeySender, msg.Sender), + sdk.NewAttribute(types.AttributeKeySender, msg.Sender), sdk.NewAttribute(types.AttributeKeyReceiver, msg.Receiver), - sdk.NewAttribute(types.AttributeKeyAmount, msg.Token.Amount.String()), - sdk.NewAttribute(types.AttributeKeyDenom, msg.Token.Denom), + sdk.NewAttribute(types.AttributeKeyTokens, coins.String()), sdk.NewAttribute(types.AttributeKeyMemo, msg.Memo), ), sdk.NewEvent( @@ -58,6 +67,8 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types. ), }) + k.Logger(ctx).Info("IBC fungible token transfer", "tokens", coins, "sender", msg.Sender, "receiver", msg.Receiver) + return &types.MsgTransferResponse{Sequence: sequence}, nil } diff --git a/modules/apps/transfer/keeper/msg_server_test.go b/modules/apps/transfer/keeper/msg_server_test.go index dfca09ceabb..4806fc23f5d 100644 --- a/modules/apps/transfer/keeper/msg_server_test.go +++ b/modules/apps/transfer/keeper/msg_server_test.go @@ -1,27 +1,62 @@ package keeper_test import ( + "errors" + "strings" + sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + abci "github.com/cometbft/cometbft/abci/types" + "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" ibctesting "github.com/cosmos/ibc-go/v8/testing" ) // TestMsgTransfer tests Transfer rpc handler func (suite *KeeperTestSuite) TestMsgTransfer() { var msg *types.MsgTransfer + var path *ibctesting.Path + var coin1 sdk.Coin + var coin2 sdk.Coin testCases := []struct { - name string - malleate func() - expPass bool + name string + malleate func() + expError error + multiDenom bool }{ { - "success", + "success: single denom", func() {}, + nil, + false, + }, + { + "success: multidenom", + func() { + coin2 = sdk.NewCoin("bond", sdkmath.NewInt(100)) + coins := sdk.NewCoins(coin1, coin2) + + // send some coins of the second denom from bank module to the sender account as well + suite.Require().NoError(suite.chainA.GetSimApp().BankKeeper.MintCoins(suite.chainA.GetContext(), types.ModuleName, sdk.NewCoins(coin2))) + suite.Require().NoError(suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, suite.chainA.SenderAccount.GetAddress(), sdk.NewCoins(coin2))) + + msg = types.NewMsgTransfer( + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + coins, + suite.chainA.SenderAccount.GetAddress().String(), + suite.chainB.SenderAccount.GetAddress().String(), + suite.chainB.GetTimeoutHeight(), 0, // only use timeout height + "memo", + ) + }, + nil, true, }, { @@ -34,10 +69,11 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { ) suite.Require().NoError(err) }, - true, + nil, + false, }, { - "send transfers disabled", + "failure: send transfers disabled", func() { suite.chainA.GetSimApp().TransferKeeper.SetParams(suite.chainA.GetContext(), types.Params{ @@ -45,24 +81,27 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { }, ) }, + types.ErrSendDisabled, false, }, { - "invalid sender", + "failure: invalid sender", func() { msg.Sender = "address" }, + errors.New("decoding bech32 failed"), false, }, { - "sender is a blocked address", + "failure: sender is a blocked address", func() { msg.Sender = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(types.ModuleName).String() }, + ibcerrors.ErrUnauthorized, false, }, { - "bank send disabled for denom", + "failure: bank send disabled for denom", func() { err := suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(), banktypes.Params{ @@ -71,15 +110,75 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { ) suite.Require().NoError(err) }, + types.ErrSendDisabled, false, }, { - "channel does not exist", + "failure: bank send disabled for coin in multi coin transfer", + func() { + coin2 = sdk.NewCoin("bond", sdkmath.NewInt(100)) + coins := sdk.NewCoins(coin1, coin2) + + // send some coins of the second denom from bank module to the sender account as well + suite.Require().NoError(suite.chainA.GetSimApp().BankKeeper.MintCoins(suite.chainA.GetContext(), types.ModuleName, sdk.NewCoins(coin2))) + suite.Require().NoError(suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, suite.chainA.SenderAccount.GetAddress(), sdk.NewCoins(coin2))) + + msg = types.NewMsgTransfer( + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + coins, + suite.chainA.SenderAccount.GetAddress().String(), + suite.chainB.SenderAccount.GetAddress().String(), + suite.chainB.GetTimeoutHeight(), 0, // only use timeout height + "memo", + ) + + err := suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(), + banktypes.Params{ + SendEnabled: []*banktypes.SendEnabled{{Denom: coin2.Denom, Enabled: false}}, + }, + ) + suite.Require().NoError(err) + }, + types.ErrSendDisabled, + true, + }, + { + "failure: channel does not exist", func() { msg.SourceChannel = "channel-100" }, + ibcerrors.ErrInvalidRequest, false, }, + { + "failure: multidenom with ics20-1", + func() { + coin2 = sdk.NewCoin("bond", sdkmath.NewInt(100)) + coins := sdk.NewCoins(coin1, coin2) + + // send some coins of the second denom from bank module to the sender account as well + suite.Require().NoError(suite.chainA.GetSimApp().BankKeeper.MintCoins(suite.chainA.GetContext(), types.ModuleName, sdk.NewCoins(coin2))) + suite.Require().NoError(suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, suite.chainA.SenderAccount.GetAddress(), sdk.NewCoins(coin2))) + + msg = types.NewMsgTransfer( + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + coins, + suite.chainA.SenderAccount.GetAddress().String(), + suite.chainB.SenderAccount.GetAddress().String(), + suite.chainB.GetTimeoutHeight(), 0, // only use timeout height + "memo", + ) + + // explicitly set to ics20-1 which does not support multi-denom + path.EndpointA.UpdateChannel(func(channel *channeltypes.Channel) { + channel.Version = types.V1 + }) + }, + ibcerrors.ErrInvalidRequest, + true, + }, } for _, tc := range testCases { @@ -88,14 +187,16 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { suite.Run(tc.name, func() { suite.SetupTest() - path := ibctesting.NewTransferPath(suite.chainA, suite.chainB) + path = ibctesting.NewTransferPath(suite.chainA, suite.chainB) path.Setup() - coin := sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100)) + coin1 = sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100)) msg = types.NewMsgTransfer( path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - coin, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), + sdk.NewCoins(coin1), + suite.chainA.SenderAccount.GetAddress().String(), + suite.chainB.SenderAccount.GetAddress().String(), suite.chainB.GetTimeoutHeight(), 0, // only use timeout height "memo", ) @@ -106,27 +207,48 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { res, err := suite.chainA.GetSimApp().TransferKeeper.Transfer(ctx, msg) // Verify events - actualEvents := ctx.EventManager().Events().ToABCIEvents() - expectedEvents := sdk.Events{ - sdk.NewEvent( - types.EventTypeTransfer, - sdk.NewAttribute(sdk.AttributeKeySender, suite.chainA.SenderAccount.GetAddress().String()), - sdk.NewAttribute(types.AttributeKeyReceiver, suite.chainB.SenderAccount.GetAddress().String()), - sdk.NewAttribute(types.AttributeKeyAmount, coin.Amount.String()), - sdk.NewAttribute(types.AttributeKeyDenom, coin.Denom), - sdk.NewAttribute(types.AttributeKeyMemo, "memo"), - ), - }.ToABCIEvents() + events := ctx.EventManager().Events().ToABCIEvents() - if tc.expPass { + var expEvents []abci.Event + if tc.multiDenom { + expEvents = sdk.Events{ + sdk.NewEvent(types.EventTypeTransfer, + sdk.NewAttribute(types.AttributeKeySender, msg.Sender), + sdk.NewAttribute(types.AttributeKeyReceiver, msg.Receiver), + sdk.NewAttribute(types.AttributeKeyMemo, msg.Memo), + sdk.NewAttribute(types.AttributeKeyTokens, sdk.NewCoins(coin1, coin2).String()), + ), + sdk.NewEvent( + sdk.EventTypeMessage, + sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), + ), + }.ToABCIEvents() + } else { + expEvents = sdk.Events{ + sdk.NewEvent(types.EventTypeTransfer, + sdk.NewAttribute(types.AttributeKeySender, msg.Sender), + sdk.NewAttribute(types.AttributeKeyReceiver, msg.Receiver), + sdk.NewAttribute(types.AttributeKeyMemo, msg.Memo), + sdk.NewAttribute(types.AttributeKeyTokens, coin1.String()), + ), + sdk.NewEvent( + sdk.EventTypeMessage, + sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), + ), + }.ToABCIEvents() + } + + expPass := tc.expError == nil + if expPass { suite.Require().NoError(err) suite.Require().NotNil(res) suite.Require().NotEqual(res.Sequence, uint64(0)) - ibctesting.AssertEvents(&suite.Suite, expectedEvents, actualEvents) + ibctesting.AssertEvents(&suite.Suite, expEvents, events) } else { - suite.Require().Error(err) suite.Require().Nil(res) - suite.Require().Len(actualEvents, 0) + suite.Require().Error(err) + suite.Require().True(errors.Is(err, tc.expError) || strings.Contains(err.Error(), tc.expError.Error()), err.Error()) + suite.Require().Len(events, 0) } }) } diff --git a/modules/apps/transfer/keeper/relay.go b/modules/apps/transfer/keeper/relay.go index 3bae0b9da03..2c6dc104fbc 100644 --- a/modules/apps/transfer/keeper/relay.go +++ b/modules/apps/transfer/keeper/relay.go @@ -12,6 +12,7 @@ import ( "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" + convertinternal "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/convert" "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" @@ -56,7 +57,7 @@ func (k Keeper) sendTransfer( ctx sdk.Context, sourcePort, sourceChannel string, - token sdk.Coin, + coins sdk.Coins, sender sdk.AccAddress, receiver string, timeoutHeight clienttypes.Height, @@ -71,6 +72,11 @@ func (k Keeper) sendTransfer( destinationPort := channel.Counterparty.PortId destinationChannel := channel.Counterparty.ChannelId + labels := []metrics.Label{ + telemetry.NewLabel(coretypes.LabelDestinationPort, destinationPort), + telemetry.NewLabel(coretypes.LabelDestinationChannel, destinationChannel), + } + // begin createOutgoingPacket logic // 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)) @@ -78,61 +84,66 @@ func (k Keeper) sendTransfer( return 0, errorsmod.Wrap(channeltypes.ErrChannelCapabilityNotFound, "module does not own channel capability") } - // NOTE: denomination and hex hash correctness checked during msg.ValidateBasic - fullDenomPath := token.Denom + var tokens []types.Token - var err error + for _, coin := range coins { + // NOTE: denomination and hex hash correctness checked during msg.ValidateBasic + fullDenomPath := coin.Denom - // deconstruct the token denomination into the denomination trace info - // to determine if the sender is the source chain - if strings.HasPrefix(token.Denom, "ibc/") { - fullDenomPath, err = k.DenomPathFromHash(ctx, token.Denom) - if err != nil { - return 0, err + var err error + + // deconstruct the token denomination into the denomination trace info + // to determine if the sender is the source chain + if strings.HasPrefix(coin.Denom, "ibc/") { + fullDenomPath, err = k.DenomPathFromHash(ctx, coin.Denom) + if err != nil { + return 0, err + } } - } - labels := []metrics.Label{ - telemetry.NewLabel(coretypes.LabelDestinationPort, destinationPort), - telemetry.NewLabel(coretypes.LabelDestinationChannel, destinationChannel), - } + // NOTE: SendTransfer simply sends the denomination as it exists on its own + // chain inside the packet data. The receiving chain will perform denom + // prefixing as necessary. - // NOTE: SendTransfer simply sends the denomination as it exists on its own - // chain inside the packet data. The receiving chain will perform denom - // prefixing as necessary. + if types.SenderChainIsSource(sourcePort, sourceChannel, fullDenomPath) { + labels = append(labels, telemetry.NewLabel(coretypes.LabelSource, "true")) - if types.SenderChainIsSource(sourcePort, sourceChannel, fullDenomPath) { - labels = append(labels, telemetry.NewLabel(coretypes.LabelSource, "true")) + // obtain the escrow address for the source channel end + escrowAddress := types.GetEscrowAddress(sourcePort, sourceChannel) + if err := k.escrowToken(ctx, sender, escrowAddress, coin); err != nil { + return 0, err + } - // obtain the escrow address for the source channel end - escrowAddress := types.GetEscrowAddress(sourcePort, sourceChannel) - if err := k.escrowToken(ctx, sender, escrowAddress, token); err != nil { - return 0, err - } + } else { + labels = append(labels, telemetry.NewLabel(coretypes.LabelSource, "false")) - } else { - labels = append(labels, telemetry.NewLabel(coretypes.LabelSource, "false")) + // transfer the coins to the module account and burn them + if err := k.bankKeeper.SendCoinsFromAccountToModule( + ctx, sender, types.ModuleName, sdk.NewCoins(coin), + ); err != nil { + return 0, err + } - // transfer the coins to the module account and burn them - if err := k.bankKeeper.SendCoinsFromAccountToModule( - ctx, sender, types.ModuleName, sdk.NewCoins(token), - ); err != nil { - return 0, err + if err := k.bankKeeper.BurnCoins( + ctx, types.ModuleName, sdk.NewCoins(coin), + ); err != nil { + // NOTE: should not happen as the module account was + // retrieved on the step above and it has enough balance + // to burn. + panic(fmt.Errorf("cannot burn coins after a successful send to a module account: %v", err)) + } } - if err := k.bankKeeper.BurnCoins( - ctx, types.ModuleName, sdk.NewCoins(token), - ); err != nil { - // NOTE: should not happen as the module account was - // retrieved on the step above and it has enough balance - // to burn. - panic(fmt.Errorf("cannot burn coins after a successful send to a module account: %v", err)) + denom, trace := convertinternal.ExtractDenomAndTraceFromV1Denom(fullDenomPath) + token := types.Token{ + Denom: denom, + Amount: coin.Amount.String(), + Trace: trace, } + tokens = append(tokens, token) } - packetData := types.NewFungibleTokenPacketData( - fullDenomPath, token.Amount.String(), sender.String(), receiver, memo, - ) + packetData := types.NewFungibleTokenPacketDataV2(tokens, sender.String(), receiver, memo) sequence, err := k.ics4Wrapper.SendPacket(ctx, channelCap, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, packetData.GetBytes()) if err != nil { @@ -140,12 +151,15 @@ func (k Keeper) sendTransfer( } defer func() { - if token.Amount.IsInt64() { - telemetry.SetGaugeWithLabels( - []string{"tx", "msg", "ibc", "transfer"}, - float32(token.Amount.Int64()), - []metrics.Label{telemetry.NewLabel(coretypes.LabelDenom, fullDenomPath)}, - ) + for _, token := range tokens { + amount, ok := sdkmath.NewIntFromString(token.Amount) + if ok && amount.IsInt64() { + telemetry.SetGaugeWithLabels( + []string{"tx", "msg", "ibc", "transfer"}, + float32(amount.Int64()), + []metrics.Label{telemetry.NewLabel(coretypes.LabelDenom, token.GetFullDenomPath())}, + ) + } } telemetry.IncrCounterWithLabels( @@ -163,7 +177,7 @@ func (k Keeper) sendTransfer( // and sent to the receiving address. Otherwise if the sender chain is sending // back tokens this chain originally transferred to it, the tokens are // unescrowed and sent to the receiving address. -func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData) error { +func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketDataV2) error { // validate packet data upon receiving if err := data.ValidateBasic(); err != nil { return errorsmod.Wrapf(err, "error validating ICS-20 transfer packet data") @@ -179,49 +193,114 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t return errorsmod.Wrapf(err, "failed to decode receiver address: %s", data.Receiver) } - // parse the transfer amount - transferAmount, ok := sdkmath.NewIntFromString(data.Amount) - if !ok { - return errorsmod.Wrapf(types.ErrInvalidAmount, "unable to parse transfer amount: %s", data.Amount) - } + for _, token := range data.Tokens { + fullDenomPath := token.GetFullDenomPath() - labels := []metrics.Label{ - telemetry.NewLabel(coretypes.LabelSourcePort, packet.GetSourcePort()), - telemetry.NewLabel(coretypes.LabelSourceChannel, packet.GetSourceChannel()), - } + labels := []metrics.Label{ + telemetry.NewLabel(coretypes.LabelSourcePort, packet.GetSourcePort()), + telemetry.NewLabel(coretypes.LabelSourceChannel, packet.GetSourceChannel()), + } - // This is the prefix that would have been prefixed to the denomination - // on sender chain IF and only if the token originally came from the - // receiving chain. - // - // NOTE: We use SourcePort and SourceChannel here, because the counterparty - // chain would have prefixed with DestPort and DestChannel when originally - // receiving this coin as seen in the "sender chain is the source" condition. - if types.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) { - // sender chain is not the source, unescrow tokens - - // remove prefix added by sender chain - voucherPrefix := types.GetDenomPrefix(packet.GetSourcePort(), packet.GetSourceChannel()) - unprefixedDenom := data.Denom[len(voucherPrefix):] - - // coin denomination used in sending from the escrow address - denom := unprefixedDenom - - // The denomination used to send the coins is either the native denom or the hash of the path - // if the denomination is not native. - denomTrace := types.ParseDenomTrace(unprefixedDenom) - if !denomTrace.IsNativeDenom() { - denom = denomTrace.IBCDenom() + // parse the transfer amount + transferAmount, ok := sdkmath.NewIntFromString(token.Amount) + if !ok { + return errorsmod.Wrapf(types.ErrInvalidAmount, "unable to parse transfer amount: %s", token.Amount) } - token := sdk.NewCoin(denom, transferAmount) - if k.bankKeeper.BlockedAddr(receiver) { - return errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "%s is not allowed to receive funds", receiver) + // This is the prefix that would have been prefixed to the denomination + // on sender chain IF and only if the token originally came from the + // receiving chain. + // + // NOTE: We use SourcePort and SourceChannel here, because the counterparty + // chain would have prefixed with DestPort and DestChannel when originally + // receiving this coin as seen in the "sender chain is the source" condition. + if types.ReceiverChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), fullDenomPath) { + // sender chain is not the source, unescrow tokens + + // remove prefix added by sender chain + voucherPrefix := types.GetDenomPrefix(packet.GetSourcePort(), packet.GetSourceChannel()) + unprefixedDenom := fullDenomPath[len(voucherPrefix):] + + // coin denomination used in sending from the escrow address + denom := unprefixedDenom + + // The denomination used to send the coins is either the native denom or the hash of the path + // if the denomination is not native. + denomTrace := types.ParseDenomTrace(unprefixedDenom) + if !denomTrace.IsNativeDenom() { + denom = denomTrace.IBCDenom() + } + token := sdk.NewCoin(denom, transferAmount) + + if k.bankKeeper.BlockedAddr(receiver) { + return errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "%s is not allowed to receive funds", receiver) + } + + escrowAddress := types.GetEscrowAddress(packet.GetDestPort(), packet.GetDestChannel()) + if err := k.unescrowToken(ctx, escrowAddress, receiver, token); err != nil { + return err + } + + defer func() { + if transferAmount.IsInt64() { + telemetry.SetGaugeWithLabels( + []string{"ibc", types.ModuleName, "packet", "receive"}, + float32(transferAmount.Int64()), + []metrics.Label{telemetry.NewLabel(coretypes.LabelDenom, unprefixedDenom)}, + ) + } + + telemetry.IncrCounterWithLabels( + []string{"ibc", types.ModuleName, "receive"}, + 1, + append( + labels, telemetry.NewLabel(coretypes.LabelSource, "true"), + ), + ) + }() + + return nil } - escrowAddress := types.GetEscrowAddress(packet.GetDestPort(), packet.GetDestChannel()) - if err := k.unescrowToken(ctx, escrowAddress, receiver, token); err != nil { - return err + // sender chain is the source, mint vouchers + + // since SendPacket did not prefix the denomination, we must prefix denomination here + prefixedDenom := types.GetPrefixedDenom(packet.GetDestPort(), packet.GetDestChannel(), fullDenomPath) + + // construct the denomination trace from the full raw denomination + denomTrace := types.ParseDenomTrace(prefixedDenom) + + traceHash := denomTrace.Hash() + if !k.HasDenomTrace(ctx, traceHash) { + k.SetDenomTrace(ctx, denomTrace) + } + + voucherDenom := denomTrace.IBCDenom() + if !k.bankKeeper.HasDenomMetaData(ctx, voucherDenom) { + k.setDenomMetadata(ctx, denomTrace) + } + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + types.EventTypeDenomTrace, + sdk.NewAttribute(types.AttributeKeyTraceHash, traceHash.String()), + sdk.NewAttribute(types.AttributeKeyDenom, voucherDenom), + ), + ) + voucher := sdk.NewCoin(voucherDenom, transferAmount) + + // mint new tokens if the source of the transfer is the same chain + if err := k.bankKeeper.MintCoins( + ctx, types.ModuleName, sdk.NewCoins(voucher), + ); err != nil { + return errorsmod.Wrap(err, "failed to mint IBC tokens") + } + + // send to receiver + if err := k.bankKeeper.SendCoinsFromModuleToAccount( + ctx, types.ModuleName, receiver, sdk.NewCoins(voucher), + ); err != nil { + return errorsmod.Wrapf(err, "failed to send coins to receiver %s", receiver.String()) } defer func() { @@ -229,7 +308,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t telemetry.SetGaugeWithLabels( []string{"ibc", types.ModuleName, "packet", "receive"}, float32(transferAmount.Int64()), - []metrics.Label{telemetry.NewLabel(coretypes.LabelDenom, unprefixedDenom)}, + []metrics.Label{telemetry.NewLabel(coretypes.LabelDenom, fullDenomPath)}, ) } @@ -237,73 +316,12 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t []string{"ibc", types.ModuleName, "receive"}, 1, append( - labels, telemetry.NewLabel(coretypes.LabelSource, "true"), + labels, telemetry.NewLabel(coretypes.LabelSource, "false"), ), ) }() - - return nil - } - - // sender chain is the source, mint vouchers - - // since SendPacket did not prefix the denomination, we must prefix denomination here - prefixedDenom := types.GetPrefixedDenom(packet.GetDestPort(), packet.GetDestChannel(), data.Denom) - - // construct the denomination trace from the full raw denomination - denomTrace := types.ParseDenomTrace(prefixedDenom) - - traceHash := denomTrace.Hash() - if !k.HasDenomTrace(ctx, traceHash) { - k.SetDenomTrace(ctx, denomTrace) } - voucherDenom := denomTrace.IBCDenom() - if !k.bankKeeper.HasDenomMetaData(ctx, voucherDenom) { - k.setDenomMetadata(ctx, denomTrace) - } - - ctx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeDenomTrace, - sdk.NewAttribute(types.AttributeKeyTraceHash, traceHash.String()), - sdk.NewAttribute(types.AttributeKeyDenom, voucherDenom), - ), - ) - voucher := sdk.NewCoin(voucherDenom, transferAmount) - - // mint new tokens if the source of the transfer is the same chain - if err := k.bankKeeper.MintCoins( - ctx, types.ModuleName, sdk.NewCoins(voucher), - ); err != nil { - return errorsmod.Wrap(err, "failed to mint IBC tokens") - } - - // send to receiver - if err := k.bankKeeper.SendCoinsFromModuleToAccount( - ctx, types.ModuleName, receiver, sdk.NewCoins(voucher), - ); err != nil { - return errorsmod.Wrapf(err, "failed to send coins to receiver %s", receiver.String()) - } - - defer func() { - if transferAmount.IsInt64() { - telemetry.SetGaugeWithLabels( - []string{"ibc", types.ModuleName, "packet", "receive"}, - float32(transferAmount.Int64()), - []metrics.Label{telemetry.NewLabel(coretypes.LabelDenom, data.Denom)}, - ) - } - - telemetry.IncrCounterWithLabels( - []string{"ibc", types.ModuleName, "receive"}, - 1, - append( - labels, telemetry.NewLabel(coretypes.LabelSource, "false"), - ), - ) - }() - return nil } @@ -311,7 +329,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t // acknowledgement written on the receiving chain. If the acknowledgement // was a success then nothing occurs. If the acknowledgement failed, then // the sender is refunded their tokens using the refundPacketToken function. -func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData, ack channeltypes.Acknowledgement) error { +func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketDataV2, ack channeltypes.Acknowledgement) error { switch ack.Response.(type) { case *channeltypes.Acknowledgement_Result: // the acknowledgement succeeded on the receiving chain so nothing @@ -326,7 +344,7 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac // OnTimeoutPacket refunds the sender since the original packet sent was // never received and has been timed out. -func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData) error { +func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketDataV2) error { return k.refundPacketToken(ctx, packet, data) } @@ -334,40 +352,44 @@ func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, dat // if the sending chain was the source chain. Otherwise, the sent tokens // were burnt in the original send so new tokens are minted and sent to // the sending address. -func (k Keeper) refundPacketToken(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketData) error { +func (k Keeper) refundPacketToken(ctx sdk.Context, packet channeltypes.Packet, data types.FungibleTokenPacketDataV2) error { // NOTE: packet data type already checked in handler.go - // parse the denomination from the full denom path - trace := types.ParseDenomTrace(data.Denom) + for _, token := range data.Tokens { + fullDenomPath := token.GetFullDenomPath() - // parse the transfer amount - transferAmount, ok := sdkmath.NewIntFromString(data.Amount) - if !ok { - return errorsmod.Wrapf(types.ErrInvalidAmount, "unable to parse transfer amount (%s) into math.Int", data.Amount) - } - token := sdk.NewCoin(trace.IBCDenom(), transferAmount) + // parse the denomination from the full denom path + trace := types.ParseDenomTrace(fullDenomPath) - // decode the sender address - sender, err := sdk.AccAddressFromBech32(data.Sender) - if err != nil { - return err - } + // parse the transfer amount + transferAmount, ok := sdkmath.NewIntFromString(token.Amount) + if !ok { + return errorsmod.Wrapf(types.ErrInvalidAmount, "unable to parse transfer amount (%s) into math.Int", transferAmount) + } + token := sdk.NewCoin(trace.IBCDenom(), transferAmount) - if types.SenderChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) { - // unescrow tokens back to sender - escrowAddress := types.GetEscrowAddress(packet.GetSourcePort(), packet.GetSourceChannel()) - return k.unescrowToken(ctx, escrowAddress, sender, token) - } + // decode the sender address + sender, err := sdk.AccAddressFromBech32(data.Sender) + if err != nil { + return err + } - // mint vouchers back to sender - if err := k.bankKeeper.MintCoins( - ctx, types.ModuleName, sdk.NewCoins(token), - ); err != nil { - return err - } + if types.SenderChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), fullDenomPath) { + // unescrow tokens back to sender + escrowAddress := types.GetEscrowAddress(packet.GetSourcePort(), packet.GetSourceChannel()) + return k.unescrowToken(ctx, escrowAddress, sender, token) + } - if err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, sender, sdk.NewCoins(token)); err != nil { - panic(fmt.Errorf("unable to send coins from module to account despite previously minting coins to module account: %v", err)) + // mint vouchers back to sender + if err := k.bankKeeper.MintCoins( + ctx, types.ModuleName, sdk.NewCoins(token), + ); err != nil { + return err + } + + if err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, sender, sdk.NewCoins(token)); err != nil { + panic(fmt.Errorf("unable to send coins from module to account despite previously minting coins to module account: %v", err)) + } } return nil diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index 821a76a877c..0f7e8a70997 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -10,6 +10,7 @@ import ( banktestutil "github.com/cosmos/cosmos-sdk/x/bank/testutil" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + convertinternal "github.com/cosmos/ibc-go/v8/modules/apps/transfer/internal/convert" "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" @@ -127,7 +128,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() { expEscrowAmount = sdkmath.ZeroInt() // create IBC token on chainA - transferMsg := types.NewMsgTransfer(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, coin, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainA.GetTimeoutHeight(), 0, "") + transferMsg := types.NewMsgTransfer(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, sdk.NewCoins(coin), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainA.GetTimeoutHeight(), 0, "") result, err := suite.chainB.SendMsgs(transferMsg) suite.Require().NoError(err) // message committed @@ -142,7 +143,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() { msg := types.NewMsgTransfer( path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - coin, sender.String(), suite.chainB.SenderAccount.GetAddress().String(), + sdk.NewCoins(coin), sender.String(), suite.chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0, // only use timeout height memo, ) @@ -207,7 +208,7 @@ func (suite *KeeperTestSuite) TestSendTransferSetsTotalEscrowAmountForSourceIBCT transferMsg := types.NewMsgTransfer( path1.EndpointA.ChannelConfig.PortID, path1.EndpointA.ChannelID, - coin, + sdk.NewCoins(coin), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainB.GetTimeoutHeight(), 0, "", @@ -227,7 +228,7 @@ func (suite *KeeperTestSuite) TestSendTransferSetsTotalEscrowAmountForSourceIBCT msg := types.NewMsgTransfer( path2.EndpointB.ChannelConfig.PortID, path2.EndpointB.ChannelID, - coin, + sdk.NewCoins(coin), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainA.GetTimeoutHeight(), 0, "", @@ -248,10 +249,10 @@ func (suite *KeeperTestSuite) TestSendTransferSetsTotalEscrowAmountForSourceIBCT // malleate function allows for testing invalid cases. func (suite *KeeperTestSuite) TestOnRecvPacket() { var ( - trace types.DenomTrace amount sdkmath.Int receiver string memo string + denomTrace types.DenomTrace expEscrowAmount sdkmath.Int // total amount in escrow for denom on receiving chain ) @@ -291,7 +292,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { { "empty coin", func() { - trace = types.DenomTrace{} + denomTrace = types.DenomTrace{} amount = sdkmath.ZeroInt() expEscrowAmount = sdkmath.NewInt(100) }, true, false, @@ -384,7 +385,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { if tc.recvIsSource { // send coin from chainB to chainA, receive them, acknowledge them, and send back to chainB coinFromBToA := sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100)) - transferMsg := types.NewMsgTransfer(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, coinFromBToA, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 110), 0, memo) + transferMsg := types.NewMsgTransfer(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, sdk.NewCoins(coinFromBToA), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 110), 0, memo) res, err := suite.chainB.SendMsgs(transferMsg) suite.Require().NoError(err) // message committed @@ -397,20 +398,28 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { seq++ // NOTE: trace must be explicitly changed in malleate to test invalid cases - trace = types.ParseDenomTrace(types.GetPrefixedDenom(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.DefaultBondDenom)) + denomTrace = types.ParseDenomTrace(types.GetPrefixedDenom(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.DefaultBondDenom)) } else { - trace = types.ParseDenomTrace(sdk.DefaultBondDenom) + denomTrace = types.ParseDenomTrace(sdk.DefaultBondDenom) } // send coin from chainA to chainB - coin := sdk.NewCoin(trace.IBCDenom(), amount) - transferMsg := types.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, coin, suite.chainA.SenderAccount.GetAddress().String(), receiver, clienttypes.NewHeight(1, 110), 0, memo) + coin := sdk.NewCoin(denomTrace.IBCDenom(), amount) + transferMsg := types.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.NewCoins(coin), suite.chainA.SenderAccount.GetAddress().String(), receiver, clienttypes.NewHeight(1, 110), 0, memo) _, err := suite.chainA.SendMsgs(transferMsg) suite.Require().NoError(err) // message committed tc.malleate() - data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.String(), suite.chainA.SenderAccount.GetAddress().String(), receiver, memo) + denom, trace := convertinternal.ExtractDenomAndTraceFromV1Denom(denomTrace.GetFullDenomPath()) + data := types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount.String(), + Trace: trace, + }, + }, suite.chainA.SenderAccount.GetAddress().String(), receiver, memo) packet := channeltypes.NewPacket(data.GetBytes(), seq, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.NewHeight(1, 100), 0) err = suite.chainB.GetSimApp().TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, data) @@ -484,12 +493,16 @@ func (suite *KeeperTestSuite) TestOnRecvPacketSetsTotalEscrowAmountForSourceIBCT BaseDenom: sdk.DefaultBondDenom, Path: fmt.Sprintf("%s/%s/%s/%s", path2.EndpointA.ChannelConfig.PortID, path2.EndpointA.ChannelID, path1.EndpointB.ChannelConfig.PortID, path1.EndpointB.ChannelID), } - data := types.NewFungibleTokenPacketData( - denomTrace.GetFullDenomPath(), - amount.String(), - suite.chainA.SenderAccount.GetAddress().String(), - suite.chainB.SenderAccount.GetAddress().String(), "", - ) + + denom, trace := convertinternal.ExtractDenomAndTraceFromV1Denom(denomTrace.GetFullDenomPath()) + data := types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount.String(), + Trace: trace, + }, + }, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), "") packet := channeltypes.NewPacket( data.GetBytes(), seq, @@ -538,7 +551,7 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() { var ( successAck = channeltypes.NewResultAcknowledgement([]byte{byte(1)}) failedAck = channeltypes.NewErrorAcknowledgement(fmt.Errorf("failed packet transfer")) - trace types.DenomTrace + denomTrace types.DenomTrace amount sdkmath.Int path *ibctesting.Path expEscrowAmount sdkmath.Int @@ -555,7 +568,7 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() { "success ack causes no-op", successAck, func() { - trace = types.ParseDenomTrace(types.GetPrefixedDenom(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, sdk.DefaultBondDenom)) + denomTrace = types.ParseDenomTrace(types.GetPrefixedDenom(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, sdk.DefaultBondDenom)) }, true, true, }, { @@ -563,7 +576,7 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() { failedAck, func() { escrow := types.GetEscrowAddress(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - trace = types.ParseDenomTrace(sdk.DefaultBondDenom) + denomTrace = types.ParseDenomTrace(sdk.DefaultBondDenom) coin := sdk.NewCoin(sdk.DefaultBondDenom, amount) suite.Require().NoError(banktestutil.FundAccount(suite.chainA.GetContext(), suite.chainA.GetSimApp().BankKeeper, escrow, sdk.NewCoins(coin))) @@ -576,7 +589,7 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() { "unsuccessful refund from source", failedAck, func() { - trace = types.ParseDenomTrace(sdk.DefaultBondDenom) + denomTrace = types.ParseDenomTrace(sdk.DefaultBondDenom) // set escrow amount that would have been stored after successful execution of MsgTransfer suite.chainA.GetSimApp().TransferKeeper.SetTotalEscrowForDenom(suite.chainA.GetContext(), sdk.NewCoin(sdk.DefaultBondDenom, amount)) @@ -588,8 +601,8 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() { failedAck, func() { escrow := types.GetEscrowAddress(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - trace = types.ParseDenomTrace(types.GetPrefixedDenom(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.DefaultBondDenom)) - coin := sdk.NewCoin(trace.IBCDenom(), amount) + denomTrace = types.ParseDenomTrace(types.GetPrefixedDenom(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.DefaultBondDenom)) + coin := sdk.NewCoin(denomTrace.IBCDenom(), amount) suite.Require().NoError(banktestutil.FundAccount(suite.chainA.GetContext(), suite.chainA.GetSimApp().BankKeeper, escrow, sdk.NewCoins(coin))) }, false, true, @@ -608,19 +621,27 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() { tc.malleate() - data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), "") + denom, trace := convertinternal.ExtractDenomAndTraceFromV1Denom(denomTrace.GetFullDenomPath()) + data := types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount.String(), + Trace: trace, + }, + }, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), "") packet := channeltypes.NewPacket(data.GetBytes(), 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.NewHeight(1, 100), 0) - preCoin := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), trace.IBCDenom()) + preCoin := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), denomTrace.IBCDenom()) err := suite.chainA.GetSimApp().TransferKeeper.OnAcknowledgementPacket(suite.chainA.GetContext(), packet, data, tc.ack) // check total amount in escrow of sent token denom on sending chain - totalEscrow := suite.chainA.GetSimApp().TransferKeeper.GetTotalEscrowForDenom(suite.chainA.GetContext(), trace.IBCDenom()) + totalEscrow := suite.chainA.GetSimApp().TransferKeeper.GetTotalEscrowForDenom(suite.chainA.GetContext(), denomTrace.IBCDenom()) suite.Require().Equal(expEscrowAmount, totalEscrow.Amount) if tc.expPass { suite.Require().NoError(err) - postCoin := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), trace.IBCDenom()) + postCoin := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), denomTrace.IBCDenom()) deltaAmount := postCoin.Amount.Sub(preCoin.Amount) if tc.success { @@ -693,11 +714,18 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacketSetsTotalEscrowAmountFo ), ) - data := types.NewFungibleTokenPacketData( - denomTrace.GetFullDenomPath(), - amount.String(), + denom, trace := convertinternal.ExtractDenomAndTraceFromV1Denom(denomTrace.GetFullDenomPath()) + data := types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount.String(), + Trace: trace, + }, + }, suite.chainB.SenderAccount.GetAddress().String(), - suite.chainA.SenderAccount.GetAddress().String(), "", + suite.chainA.SenderAccount.GetAddress().String(), + "", ) packet := channeltypes.NewPacket( data.GetBytes(), @@ -727,10 +755,10 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacketSetsTotalEscrowAmountFo // so the refunds are occurring on chainA. func (suite *KeeperTestSuite) TestOnTimeoutPacket() { var ( - trace types.DenomTrace path *ibctesting.Path amount sdkmath.Int sender string + denomTrace types.DenomTrace expEscrowAmount sdkmath.Int ) @@ -743,8 +771,8 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacket() { "successful timeout from sender as source chain", func() { escrow := types.GetEscrowAddress(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - trace = types.ParseDenomTrace(sdk.DefaultBondDenom) - coin := sdk.NewCoin(trace.IBCDenom(), amount) + denomTrace = types.ParseDenomTrace(sdk.DefaultBondDenom) + coin := sdk.NewCoin(denomTrace.IBCDenom(), amount) expEscrowAmount = sdkmath.ZeroInt() // funds the escrow account to have balance @@ -757,8 +785,8 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacket() { "successful timeout from external chain", func() { escrow := types.GetEscrowAddress(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - trace = types.ParseDenomTrace(types.GetPrefixedDenom(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.DefaultBondDenom)) - coin := sdk.NewCoin(trace.IBCDenom(), amount) + denomTrace = types.ParseDenomTrace(types.GetPrefixedDenom(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.DefaultBondDenom)) + coin := sdk.NewCoin(denomTrace.IBCDenom(), amount) expEscrowAmount = sdkmath.ZeroInt() // funds the escrow account to have balance @@ -768,27 +796,27 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacket() { { "no balance for coin denom", func() { - trace = types.ParseDenomTrace("bitcoin") + denomTrace = types.ParseDenomTrace("bitcoin") expEscrowAmount = amount // set escrow amount that would have been stored after successful execution of MsgTransfer - suite.chainA.GetSimApp().TransferKeeper.SetTotalEscrowForDenom(suite.chainA.GetContext(), sdk.NewCoin(trace.IBCDenom(), amount)) + suite.chainA.GetSimApp().TransferKeeper.SetTotalEscrowForDenom(suite.chainA.GetContext(), sdk.NewCoin(denomTrace.IBCDenom(), amount)) }, false, }, { "unescrow failed", func() { - trace = types.ParseDenomTrace(sdk.DefaultBondDenom) + denomTrace = types.ParseDenomTrace(sdk.DefaultBondDenom) expEscrowAmount = amount // set escrow amount that would have been stored after successful execution of MsgTransfer - suite.chainA.GetSimApp().TransferKeeper.SetTotalEscrowForDenom(suite.chainA.GetContext(), sdk.NewCoin(trace.IBCDenom(), amount)) + suite.chainA.GetSimApp().TransferKeeper.SetTotalEscrowForDenom(suite.chainA.GetContext(), sdk.NewCoin(denomTrace.IBCDenom(), amount)) }, false, }, { "mint failed", func() { - trace = types.ParseDenomTrace(types.GetPrefixedDenom(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.DefaultBondDenom)) + denomTrace = types.ParseDenomTrace(types.GetPrefixedDenom(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.DefaultBondDenom)) amount = sdkmath.OneInt() sender = "invalid address" }, false, @@ -809,17 +837,25 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacket() { tc.malleate() - data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.String(), sender, suite.chainB.SenderAccount.GetAddress().String(), "") + denom, trace := convertinternal.ExtractDenomAndTraceFromV1Denom(denomTrace.GetFullDenomPath()) + data := types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount.String(), + Trace: trace, + }, + }, sender, suite.chainB.SenderAccount.GetAddress().String(), "") packet := channeltypes.NewPacket(data.GetBytes(), 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.NewHeight(1, 100), 0) - preCoin := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), trace.IBCDenom()) + preCoin := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), denomTrace.IBCDenom()) err := suite.chainA.GetSimApp().TransferKeeper.OnTimeoutPacket(suite.chainA.GetContext(), packet, data) - postCoin := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), trace.IBCDenom()) + postCoin := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), denomTrace.IBCDenom()) deltaAmount := postCoin.Amount.Sub(preCoin.Amount) // check total amount in escrow of sent token denom on sending chain - totalEscrow := suite.chainA.GetSimApp().TransferKeeper.GetTotalEscrowForDenom(suite.chainA.GetContext(), trace.IBCDenom()) + totalEscrow := suite.chainA.GetSimApp().TransferKeeper.GetTotalEscrowForDenom(suite.chainA.GetContext(), denomTrace.IBCDenom()) suite.Require().Equal(expEscrowAmount, totalEscrow.Amount) if tc.expPass { @@ -887,12 +923,15 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacketSetsTotalEscrowAmountForSourceI ), ) - data := types.NewFungibleTokenPacketData( - denomTrace.GetFullDenomPath(), - amount.String(), - suite.chainB.SenderAccount.GetAddress().String(), - suite.chainA.SenderAccount.GetAddress().String(), "", - ) + denom, trace := convertinternal.ExtractDenomAndTraceFromV1Denom(denomTrace.GetFullDenomPath()) + data := types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount.String(), + Trace: trace, + }, + }, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), "") packet := channeltypes.NewPacket( data.GetBytes(), seq, @@ -957,7 +996,7 @@ func (suite *KeeperTestSuite) TestPacketForwardsCompatibility() { func() { packetData = []byte("invalid packet data") }, - ibcerrors.ErrUnknownRequest, + ibcerrors.ErrInvalidType, }, { "failure: missing field", @@ -965,7 +1004,7 @@ func (suite *KeeperTestSuite) TestPacketForwardsCompatibility() { jsonString := fmt.Sprintf(`{"amount":"100","sender":%s","receiver":"%s"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) packetData = []byte(jsonString) }, - ibcerrors.ErrUnknownRequest, + ibcerrors.ErrInvalidType, }, } @@ -976,6 +1015,8 @@ func (suite *KeeperTestSuite) TestPacketForwardsCompatibility() { packetData = nil path := ibctesting.NewTransferPath(suite.chainA, suite.chainB) + path.EndpointA.ChannelConfig.Version = types.V1 + path.EndpointB.ChannelConfig.Version = types.V1 path.Setup() tc.malleate() diff --git a/modules/apps/transfer/transfer_test.go b/modules/apps/transfer/transfer_test.go index ef8dea295c6..1cb1261c0e3 100644 --- a/modules/apps/transfer/transfer_test.go +++ b/modules/apps/transfer/transfer_test.go @@ -52,7 +52,7 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() { coinToSendToB := sdk.NewCoin(sdk.DefaultBondDenom, amount) // send from chainA to chainB - msg := types.NewMsgTransfer(pathAtoB.EndpointA.ChannelConfig.PortID, pathAtoB.EndpointA.ChannelID, coinToSendToB, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0, "") + msg := types.NewMsgTransfer(pathAtoB.EndpointA.ChannelConfig.PortID, pathAtoB.EndpointA.ChannelID, sdk.NewCoins(coinToSendToB), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0, "") res, err := suite.chainA.SendMsgs(msg) suite.Require().NoError(err) // message committed @@ -82,7 +82,7 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() { pathBtoC.Setup() // send from chainB to chainC - msg = types.NewMsgTransfer(pathBtoC.EndpointA.ChannelConfig.PortID, pathBtoC.EndpointA.ChannelID, coinSentFromAToB, suite.chainB.SenderAccount.GetAddress().String(), suite.chainC.SenderAccount.GetAddress().String(), timeoutHeight, 0, "") + msg = types.NewMsgTransfer(pathBtoC.EndpointA.ChannelConfig.PortID, pathBtoC.EndpointA.ChannelID, sdk.NewCoins(coinSentFromAToB), suite.chainB.SenderAccount.GetAddress().String(), suite.chainC.SenderAccount.GetAddress().String(), timeoutHeight, 0, "") res, err = suite.chainB.SendMsgs(msg) suite.Require().NoError(err) // message committed @@ -105,7 +105,7 @@ func (suite *TransferTestSuite) TestHandleMsgTransfer() { suite.Require().Zero(balance.Amount.Int64()) // send from chainC back to chainB - msg = types.NewMsgTransfer(pathBtoC.EndpointB.ChannelConfig.PortID, pathBtoC.EndpointB.ChannelID, coinSentFromBToC, suite.chainC.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0, "") + msg = types.NewMsgTransfer(pathBtoC.EndpointB.ChannelConfig.PortID, pathBtoC.EndpointB.ChannelID, sdk.NewCoins(coinSentFromBToC), suite.chainC.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), timeoutHeight, 0, "") res, err = suite.chainC.SendMsgs(msg) suite.Require().NoError(err) // message committed diff --git a/modules/apps/transfer/types/events.go b/modules/apps/transfer/types/events.go index 89964a8a9a4..f310ba5d0d9 100644 --- a/modules/apps/transfer/types/events.go +++ b/modules/apps/transfer/types/events.go @@ -8,12 +8,12 @@ const ( EventTypeChannelClose = "channel_closed" EventTypeDenomTrace = "denomination_trace" + AttributeKeySender = "sender" AttributeKeyReceiver = "receiver" AttributeKeyDenom = "denom" - AttributeKeyAmount = "amount" + AttributeKeyTokens = "tokens" AttributeKeyRefundReceiver = "refund_receiver" - AttributeKeyRefundDenom = "refund_denom" - AttributeKeyRefundAmount = "refund_amount" + AttributeKeyRefundTokens = "refund_tokens" AttributeKeyAckSuccess = "success" AttributeKeyAck = "acknowledgement" AttributeKeyAckError = "error" diff --git a/modules/apps/transfer/types/expected_keepers.go b/modules/apps/transfer/types/expected_keepers.go index e639a84606f..9ed02474405 100644 --- a/modules/apps/transfer/types/expected_keepers.go +++ b/modules/apps/transfer/types/expected_keepers.go @@ -27,7 +27,7 @@ type BankKeeper interface { SendCoinsFromModuleToAccount(ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error SendCoinsFromAccountToModule(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error BlockedAddr(addr sdk.AccAddress) bool - IsSendEnabledCoin(ctx context.Context, coin sdk.Coin) bool + IsSendEnabledCoins(ctx context.Context, coins ...sdk.Coin) error HasDenomMetaData(ctx context.Context, denom string) bool SetDenomMetaData(ctx context.Context, denomMetaData banktypes.Metadata) GetBalance(ctx context.Context, addr sdk.AccAddress, denom string) sdk.Coin diff --git a/modules/apps/transfer/types/export_test.go b/modules/apps/transfer/types/export_test.go new file mode 100644 index 00000000000..65e136757fe --- /dev/null +++ b/modules/apps/transfer/types/export_test.go @@ -0,0 +1,6 @@ +package types + +// ValidateIBCDenom is a wrapper around validateIBCDenom for testing purposes. +func ValidateIBCDenom(denom string) error { + return validateIBCDenom(denom) +} diff --git a/modules/apps/transfer/types/keys.go b/modules/apps/transfer/types/keys.go index b94caa797dc..f8faa1336fe 100644 --- a/modules/apps/transfer/types/keys.go +++ b/modules/apps/transfer/types/keys.go @@ -11,10 +11,6 @@ const ( // ModuleName defines the IBC transfer name ModuleName = "transfer" - // Version defines the current version the IBC transfer - // module supports - Version = "ics20-1" - // PortID is the default port id that transfer module binds to PortID = "transfer" @@ -38,11 +34,28 @@ const ( ParamsKey = "params" ) +const ( + // V1 defines first version of the IBC transfer module + V1 = "ics20-1" + + // V2 defines the transfer version which introduces multidenom support + // through the FungibleTokenPacketDataV2. It is the latest version. + V2 = "ics20-2" + + // escrowAddressVersion should remain as ics20-1 to avoid the address changing. + // this address has been reasoned about to avoid collisions with other addresses + // https://github.com/cosmos/cosmos-sdk/issues/7737#issuecomment-735671951 + escrowAddressVersion = V1 +) + var ( // PortKey defines the key to store the port ID in store PortKey = []byte{0x01} // DenomTraceKey defines the key to store the denomination trace info in store DenomTraceKey = []byte{0x02} + + // SupportedVersions defines all versions that are supported by the module + SupportedVersions = []string{V2, V1} ) // GetEscrowAddress returns the escrow address for the specified channel. @@ -54,7 +67,7 @@ func GetEscrowAddress(portID, channelID string) sdk.AccAddress { contents := fmt.Sprintf("%s/%s", portID, channelID) // ADR 028 AddressHash construction - preImage := []byte(Version) + preImage := []byte(escrowAddressVersion) preImage = append(preImage, 0) preImage = append(preImage, contents...) hash := sha256.Sum256(preImage) diff --git a/modules/apps/transfer/types/msgs.go b/modules/apps/transfer/types/msgs.go index 2b045fee47e..c5fd206d2c4 100644 --- a/modules/apps/transfer/types/msgs.go +++ b/modules/apps/transfer/types/msgs.go @@ -15,6 +15,7 @@ import ( const ( MaximumReceiverLength = 2048 // maximum length of the receiver address in bytes (value chosen arbitrarily) MaximumMemoLength = 32768 // maximum length of the memo in bytes (value chosen arbitrarily) + MaximumTokensLength = 100 // maximum number of tokens that can be transferred in a single message (value chosen arbitrarily) ) var ( @@ -45,19 +46,19 @@ func (msg MsgUpdateParams) ValidateBasic() error { // NewMsgTransfer creates a new MsgTransfer instance func NewMsgTransfer( sourcePort, sourceChannel string, - token sdk.Coin, sender, receiver string, + tokens sdk.Coins, sender, receiver string, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, memo string, ) *MsgTransfer { return &MsgTransfer{ SourcePort: sourcePort, SourceChannel: sourceChannel, - Token: token, Sender: sender, Receiver: receiver, TimeoutHeight: timeoutHeight, TimeoutTimestamp: timeoutTimestamp, Memo: memo, + Tokens: tokens, } } @@ -72,11 +73,17 @@ func (msg MsgTransfer) ValidateBasic() error { if err := host.ChannelIdentifierValidator(msg.SourceChannel); err != nil { return errorsmod.Wrap(err, "invalid source channel ID") } - if !msg.Token.IsValid() { - return errorsmod.Wrap(ibcerrors.ErrInvalidCoins, msg.Token.String()) + + if len(msg.Tokens) == 0 && !isValidIBCCoin(msg.Token) { + return errorsmod.Wrap(ibcerrors.ErrInvalidCoins, "either token or token array must be filled") } - if !msg.Token.IsPositive() { - return errorsmod.Wrap(ibcerrors.ErrInsufficientFunds, msg.Token.String()) + + if len(msg.Tokens) != 0 && isValidIBCCoin(msg.Token) { + return errorsmod.Wrap(ibcerrors.ErrInvalidCoins, "cannot fill both token and token array") + } + + if len(msg.Tokens) > MaximumTokensLength { + return errorsmod.Wrapf(ibcerrors.ErrInvalidCoins, "number of tokens must not exceed %d", MaximumTokensLength) } _, err := sdk.AccAddressFromBech32(msg.Sender) @@ -92,5 +99,46 @@ func (msg MsgTransfer) ValidateBasic() error { if len(msg.Memo) > MaximumMemoLength { return errorsmod.Wrapf(ErrInvalidMemo, "memo must not exceed %d bytes", MaximumMemoLength) } - return ValidateIBCDenom(msg.Token.Denom) + + for _, coin := range msg.GetCoins() { + if err := validateIBCCoin(coin); err != nil { + return errorsmod.Wrapf(ibcerrors.ErrInvalidCoins, "%s: %s", err.Error(), coin.String()) + } + } + + return nil +} + +// GetCoins returns the tokens which will be transferred. +// If MsgTransfer is populated in the Token field, only that field +// will be returned in the coin array. +func (msg MsgTransfer) GetCoins() sdk.Coins { + coins := msg.Tokens + if isValidIBCCoin(msg.Token) { + coins = []sdk.Coin{msg.Token} + } + return coins +} + +// isValidIBCCoin returns true if the token provided is valid, +// and should be used to transfer tokens. +func isValidIBCCoin(coin sdk.Coin) bool { + return validateIBCCoin(coin) == nil +} + +// validateIBCCoin returns true if the token provided is valid, +// and should be used to transfer tokens. The token must +// have a positive amount. +func validateIBCCoin(coin sdk.Coin) error { + if err := coin.Validate(); err != nil { + return err + } + if !coin.IsPositive() { + return errorsmod.Wrap(ErrInvalidAmount, "amount must be positive") + } + if err := validateIBCDenom(coin.GetDenom()); err != nil { + return errorsmod.Wrap(ErrInvalidDenomForTransfer, err.Error()) + } + + return nil } diff --git a/modules/apps/transfer/types/msgs_test.go b/modules/apps/transfer/types/msgs_test.go index e9bb11cac54..f05ad3cdc68 100644 --- a/modules/apps/transfer/types/msgs_test.go +++ b/modules/apps/transfer/types/msgs_test.go @@ -14,6 +14,8 @@ import ( "github.com/cosmos/ibc-go/v8/modules/apps/transfer" "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + host "github.com/cosmos/ibc-go/v8/modules/core/24-host" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" ibctesting "github.com/cosmos/ibc-go/v8/testing" ) @@ -38,11 +40,11 @@ var ( receiver = sdk.AccAddress("testaddr2").String() emptyAddr string - coin = sdk.NewCoin("atom", sdkmath.NewInt(100)) - ibcCoin = sdk.NewCoin("ibc/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2", sdkmath.NewInt(100)) - invalidIBCCoin = sdk.NewCoin("ibc/7F1D3FCF4AE79E1554", sdkmath.NewInt(100)) - invalidDenomCoin = sdk.Coin{Denom: "0atom", Amount: sdkmath.NewInt(100)} - zeroCoin = sdk.Coin{Denom: "atoms", Amount: sdkmath.NewInt(0)} + coins = sdk.NewCoins(sdk.NewCoin("atom", sdkmath.NewInt(100))) + ibcCoins = sdk.NewCoins(sdk.NewCoin("ibc/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2", sdkmath.NewInt(100))) + invalidIBCCoins = sdk.NewCoins(sdk.NewCoin("ibc/7F1D3FCF4AE79E1554", sdkmath.NewInt(100))) + invalidDenomCoins = []sdk.Coin{{Denom: "0atom", Amount: sdkmath.NewInt(100)}} + zeroCoins = []sdk.Coin{{Denom: "atoms", Amount: sdkmath.NewInt(0)}} timeoutHeight = clienttypes.NewHeight(0, 10) ) @@ -50,36 +52,43 @@ var ( // TestMsgTransferValidation tests ValidateBasic for MsgTransfer func TestMsgTransferValidation(t *testing.T) { testCases := []struct { - name string - msg *types.MsgTransfer - expPass bool + name string + msg *types.MsgTransfer + expError error }{ - {"valid msg with base denom", types.NewMsgTransfer(validPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ""), true}, - {"valid msg with trace hash", types.NewMsgTransfer(validPort, validChannel, ibcCoin, sender, receiver, timeoutHeight, 0, ""), true}, - {"invalid ibc denom", types.NewMsgTransfer(validPort, validChannel, invalidIBCCoin, sender, receiver, timeoutHeight, 0, ""), false}, - {"too short port id", types.NewMsgTransfer(invalidShortPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ""), false}, - {"too long port id", types.NewMsgTransfer(invalidLongPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ""), false}, - {"port id contains non-alpha", types.NewMsgTransfer(invalidPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ""), false}, - {"too short channel id", types.NewMsgTransfer(validPort, invalidShortChannel, coin, sender, receiver, timeoutHeight, 0, ""), false}, - {"too long channel id", types.NewMsgTransfer(validPort, invalidLongChannel, coin, sender, receiver, timeoutHeight, 0, ""), false}, - {"too long memo", types.NewMsgTransfer(validPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ibctesting.GenerateString(types.MaximumMemoLength+1)), false}, - {"channel id contains non-alpha", types.NewMsgTransfer(validPort, invalidChannel, coin, sender, receiver, timeoutHeight, 0, ""), false}, - {"invalid denom", types.NewMsgTransfer(validPort, validChannel, invalidDenomCoin, sender, receiver, timeoutHeight, 0, ""), false}, - {"zero coin", types.NewMsgTransfer(validPort, validChannel, zeroCoin, sender, receiver, timeoutHeight, 0, ""), false}, - {"missing sender address", types.NewMsgTransfer(validPort, validChannel, coin, emptyAddr, receiver, timeoutHeight, 0, ""), false}, - {"missing recipient address", types.NewMsgTransfer(validPort, validChannel, coin, sender, "", timeoutHeight, 0, ""), false}, - {"too long recipient address", types.NewMsgTransfer(validPort, validChannel, coin, sender, ibctesting.GenerateString(types.MaximumReceiverLength+1), timeoutHeight, 0, ""), false}, - {"empty coin", types.NewMsgTransfer(validPort, validChannel, sdk.Coin{}, sender, receiver, timeoutHeight, 0, ""), false}, + {"valid msg with base denom", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, ""), nil}, + {"valid msg with trace hash", types.NewMsgTransfer(validPort, validChannel, ibcCoins, sender, receiver, timeoutHeight, 0, ""), nil}, + {"multidenom", types.NewMsgTransfer(validPort, validChannel, coins.Add(ibcCoins...), sender, receiver, timeoutHeight, 0, ""), nil}, + {"invalid ibc denom", types.NewMsgTransfer(validPort, validChannel, invalidIBCCoins, sender, receiver, timeoutHeight, 0, ""), ibcerrors.ErrInvalidCoins}, + {"too short port id", types.NewMsgTransfer(invalidShortPort, validChannel, coins, sender, receiver, timeoutHeight, 0, ""), host.ErrInvalidID}, + {"too long port id", types.NewMsgTransfer(invalidLongPort, validChannel, coins, sender, receiver, timeoutHeight, 0, ""), host.ErrInvalidID}, + {"port id contains non-alpha", types.NewMsgTransfer(invalidPort, validChannel, coins, sender, receiver, timeoutHeight, 0, ""), host.ErrInvalidID}, + {"too short channel id", types.NewMsgTransfer(validPort, invalidShortChannel, coins, sender, receiver, timeoutHeight, 0, ""), host.ErrInvalidID}, + {"too long channel id", types.NewMsgTransfer(validPort, invalidLongChannel, coins, sender, receiver, timeoutHeight, 0, ""), host.ErrInvalidID}, + {"too long memo", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, ibctesting.GenerateString(types.MaximumMemoLength+1)), types.ErrInvalidMemo}, + {"channel id contains non-alpha", types.NewMsgTransfer(validPort, invalidChannel, coins, sender, receiver, timeoutHeight, 0, ""), host.ErrInvalidID}, + {"invalid denom", types.NewMsgTransfer(validPort, validChannel, invalidDenomCoins, sender, receiver, timeoutHeight, 0, ""), ibcerrors.ErrInvalidCoins}, + {"zero coins", types.NewMsgTransfer(validPort, validChannel, zeroCoins, sender, receiver, timeoutHeight, 0, ""), ibcerrors.ErrInvalidCoins}, + {"missing sender address", types.NewMsgTransfer(validPort, validChannel, coins, emptyAddr, receiver, timeoutHeight, 0, ""), ibcerrors.ErrInvalidAddress}, + {"missing recipient address", types.NewMsgTransfer(validPort, validChannel, coins, sender, "", timeoutHeight, 0, ""), ibcerrors.ErrInvalidAddress}, + {"too long recipient address", types.NewMsgTransfer(validPort, validChannel, coins, sender, ibctesting.GenerateString(types.MaximumReceiverLength+1), timeoutHeight, 0, ""), ibcerrors.ErrInvalidAddress}, + {"empty coins", types.NewMsgTransfer(validPort, validChannel, sdk.NewCoins(), sender, receiver, timeoutHeight, 0, ""), ibcerrors.ErrInvalidCoins}, + {"multidenom: invalid denom", types.NewMsgTransfer(validPort, validChannel, coins.Add(invalidDenomCoins...), sender, receiver, timeoutHeight, 0, ""), ibcerrors.ErrInvalidCoins}, + {"multidenom: invalid ibc denom", types.NewMsgTransfer(validPort, validChannel, coins.Add(invalidIBCCoins...), sender, receiver, timeoutHeight, 0, ""), ibcerrors.ErrInvalidCoins}, + {"multidenom: zero coins", types.NewMsgTransfer(validPort, validChannel, zeroCoins, sender, receiver, timeoutHeight, 0, ""), ibcerrors.ErrInvalidCoins}, + {"multidenom: too many coins", types.NewMsgTransfer(validPort, validChannel, make([]sdk.Coin, types.MaximumTokensLength+1), sender, receiver, timeoutHeight, 0, ""), ibcerrors.ErrInvalidCoins}, } - for i, tc := range testCases { + for _, tc := range testCases { tc := tc err := tc.msg.ValidateBasic() - if tc.expPass { - require.NoError(t, err, "valid test case %d failed: %s", i, tc.name) + + expPass := tc.expError == nil + if expPass { + require.NoError(t, err) } else { - require.Error(t, err, "invalid test case %d passed: %s", i, tc.name) + require.ErrorIs(t, err, tc.expError) } } } @@ -87,7 +96,7 @@ func TestMsgTransferValidation(t *testing.T) { // TestMsgTransferGetSigners tests GetSigners for MsgTransfer func TestMsgTransferGetSigners(t *testing.T) { addr := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) - msg := types.NewMsgTransfer(validPort, validChannel, coin, addr.String(), receiver, timeoutHeight, 0, "") + msg := types.NewMsgTransfer(validPort, validChannel, coins, addr.String(), receiver, timeoutHeight, 0, "") encodingCfg := moduletestutil.MakeTestEncodingConfig(transfer.AppModuleBasic{}) signers, _, err := encodingCfg.Codec.GetMsgV1Signers(msg) @@ -98,23 +107,25 @@ func TestMsgTransferGetSigners(t *testing.T) { // TestMsgUpdateParamsValidateBasic tests ValidateBasic for MsgUpdateParams func TestMsgUpdateParamsValidateBasic(t *testing.T) { testCases := []struct { - name string - msg *types.MsgUpdateParams - expPass bool + name string + msg *types.MsgUpdateParams + expError error }{ - {"success: valid signer and valid params", types.NewMsgUpdateParams(ibctesting.TestAccAddress, types.DefaultParams()), true}, - {"failure: invalid signer with valid params", types.NewMsgUpdateParams(invalidAddress, types.DefaultParams()), false}, - {"failure: empty signer with valid params", types.NewMsgUpdateParams(emptyAddr, types.DefaultParams()), false}, + {"success: valid signer and valid params", types.NewMsgUpdateParams(ibctesting.TestAccAddress, types.DefaultParams()), nil}, + {"failure: invalid signer with valid params", types.NewMsgUpdateParams(invalidAddress, types.DefaultParams()), ibcerrors.ErrInvalidAddress}, + {"failure: empty signer with valid params", types.NewMsgUpdateParams(emptyAddr, types.DefaultParams()), ibcerrors.ErrInvalidAddress}, } - for i, tc := range testCases { + for _, tc := range testCases { tc := tc err := tc.msg.ValidateBasic() - if tc.expPass { - require.NoError(t, err, "valid test case %d failed: %s", i, tc.name) + + expPass := tc.expError == nil + if expPass { + require.NoError(t, err) } else { - require.Error(t, err, "invalid test case %d passed: %s", i, tc.name) + require.ErrorIs(t, err, tc.expError) } } } @@ -140,6 +151,7 @@ func TestMsgUpdateParamsGetSigners(t *testing.T) { encodingCfg := moduletestutil.MakeTestEncodingConfig(transfer.AppModuleBasic{}) signers, _, err := encodingCfg.Codec.GetMsgV1Signers(&msg) + if tc.expPass { require.NoError(t, err) require.Equal(t, tc.address.Bytes(), signers[0]) diff --git a/modules/apps/transfer/types/packet.go b/modules/apps/transfer/types/packet.go index 351500abfe1..e706084d56a 100644 --- a/modules/apps/transfer/types/packet.go +++ b/modules/apps/transfer/types/packet.go @@ -96,3 +96,89 @@ func (ftpd FungibleTokenPacketData) GetCustomPacketData(key string) interface{} return memoData } + +// NewFungibleTokenPacketDataV2 constructs a new FungibleTokenPacketDataV2 instance +func NewFungibleTokenPacketDataV2( + tokens []Token, + sender, receiver string, + memo string, +) FungibleTokenPacketDataV2 { + return FungibleTokenPacketDataV2{ + Tokens: tokens, + Sender: sender, + Receiver: receiver, + Memo: memo, + } +} + +// ValidateBasic is used for validating the token transfer. +// NOTE: The addresses formats are not validated as the sender and recipient can have different +// formats defined by their corresponding chains that are not known to IBC. +func (ftpd FungibleTokenPacketDataV2) ValidateBasic() error { + if strings.TrimSpace(ftpd.Sender) == "" { + return errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "sender address cannot be blank") + } + + if strings.TrimSpace(ftpd.Receiver) == "" { + return errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "receiver address cannot be blank") + } + + if len(ftpd.Tokens) == 0 { + return errorsmod.Wrap(ErrInvalidAmount, "tokens cannot be empty") + } + + for _, token := range ftpd.Tokens { + if err := token.Validate(); err != nil { + return err + } + } + + if len(ftpd.Memo) > MaximumMemoLength { + return errorsmod.Wrapf(ErrInvalidMemo, "memo must not exceed %d bytes", MaximumMemoLength) + } + + return nil +} + +// GetBytes is a helper for serialising +func (ftpd FungibleTokenPacketDataV2) GetBytes() []byte { + bz, err := json.Marshal(&ftpd) + if err != nil { + panic(errors.New("cannot marshal FungibleTokenPacketDataV2 into bytes")) + } + + return bz +} + +// GetCustomPacketData interprets the memo field of the packet data as a JSON object +// and returns the value associated with the given key. +// If the key is missing or the memo is not properly formatted, then nil is returned. +func (ftpd FungibleTokenPacketDataV2) GetCustomPacketData(key string) interface{} { + if len(ftpd.Memo) == 0 { + return nil + } + + jsonObject := make(map[string]interface{}) + err := json.Unmarshal([]byte(ftpd.Memo), &jsonObject) + if err != nil { + return nil + } + + memoData, found := jsonObject[key] + if !found { + return nil + } + + return memoData +} + +// GetPacketSender returns the sender address embedded in the packet data. +// +// NOTE: +// - The sender address is set by the module which requested the packet to be sent, +// and this module may not have validated the sender address by a signature check. +// - The sender address must only be used by modules on the sending chain. +// - sourcePortID is not used in this implementation. +func (ftpd FungibleTokenPacketDataV2) GetPacketSender(sourcePortID string) string { + return ftpd.Sender +} diff --git a/modules/apps/transfer/types/packet.pb.go b/modules/apps/transfer/types/packet.pb.go index 5b0c659e69c..94ef3c4a5a5 100644 --- a/modules/apps/transfer/types/packet.pb.go +++ b/modules/apps/transfer/types/packet.pb.go @@ -5,6 +5,7 @@ package types import ( fmt "fmt" + _ "github.com/cosmos/gogoproto/gogoproto" proto "github.com/cosmos/gogoproto/proto" io "io" math "math" @@ -106,8 +107,149 @@ func (m *FungibleTokenPacketData) GetMemo() string { return "" } +// FungibleTokenPacketDataV2 defines a struct for the packet payload +// See FungibleTokenPacketDataV2 spec: +// https://github.com/cosmos/ibc/tree/master/spec/app/ics-020-fungible-token-transfer#data-structures +type FungibleTokenPacketDataV2 struct { + // the tokens to be transferred + Tokens []Token `protobuf:"bytes,1,rep,name=tokens,proto3" json:"tokens"` + // the sender address + Sender string `protobuf:"bytes,2,opt,name=sender,proto3" json:"sender,omitempty"` + // the recipient address on the destination chain + Receiver string `protobuf:"bytes,3,opt,name=receiver,proto3" json:"receiver,omitempty"` + // optional memo + Memo string `protobuf:"bytes,4,opt,name=memo,proto3" json:"memo,omitempty"` +} + +func (m *FungibleTokenPacketDataV2) Reset() { *m = FungibleTokenPacketDataV2{} } +func (m *FungibleTokenPacketDataV2) String() string { return proto.CompactTextString(m) } +func (*FungibleTokenPacketDataV2) ProtoMessage() {} +func (*FungibleTokenPacketDataV2) Descriptor() ([]byte, []int) { + return fileDescriptor_653ca2ce9a5ca313, []int{1} +} +func (m *FungibleTokenPacketDataV2) XXX_Unmarshal(b []byte) error { + return m.Unmarshal(b) +} +func (m *FungibleTokenPacketDataV2) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { + if deterministic { + return xxx_messageInfo_FungibleTokenPacketDataV2.Marshal(b, m, deterministic) + } else { + b = b[:cap(b)] + n, err := m.MarshalToSizedBuffer(b) + if err != nil { + return nil, err + } + return b[:n], nil + } +} +func (m *FungibleTokenPacketDataV2) XXX_Merge(src proto.Message) { + xxx_messageInfo_FungibleTokenPacketDataV2.Merge(m, src) +} +func (m *FungibleTokenPacketDataV2) XXX_Size() int { + return m.Size() +} +func (m *FungibleTokenPacketDataV2) XXX_DiscardUnknown() { + xxx_messageInfo_FungibleTokenPacketDataV2.DiscardUnknown(m) +} + +var xxx_messageInfo_FungibleTokenPacketDataV2 proto.InternalMessageInfo + +func (m *FungibleTokenPacketDataV2) GetTokens() []Token { + if m != nil { + return m.Tokens + } + return nil +} + +func (m *FungibleTokenPacketDataV2) GetSender() string { + if m != nil { + return m.Sender + } + return "" +} + +func (m *FungibleTokenPacketDataV2) GetReceiver() string { + if m != nil { + return m.Receiver + } + return "" +} + +func (m *FungibleTokenPacketDataV2) GetMemo() string { + if m != nil { + return m.Memo + } + return "" +} + +// Token defines a struct which represents a token to be transferred. +type Token struct { + // the base token denomination to be transferred + Denom string `protobuf:"bytes,1,opt,name=denom,proto3" json:"denom,omitempty"` + // the token amount to be transferred + Amount string `protobuf:"bytes,2,opt,name=amount,proto3" json:"amount,omitempty"` + // the trace of the token + Trace []string `protobuf:"bytes,3,rep,name=trace,proto3" json:"trace,omitempty"` +} + +func (m *Token) Reset() { *m = Token{} } +func (m *Token) String() string { return proto.CompactTextString(m) } +func (*Token) ProtoMessage() {} +func (*Token) Descriptor() ([]byte, []int) { + return fileDescriptor_653ca2ce9a5ca313, []int{2} +} +func (m *Token) XXX_Unmarshal(b []byte) error { + return m.Unmarshal(b) +} +func (m *Token) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { + if deterministic { + return xxx_messageInfo_Token.Marshal(b, m, deterministic) + } else { + b = b[:cap(b)] + n, err := m.MarshalToSizedBuffer(b) + if err != nil { + return nil, err + } + return b[:n], nil + } +} +func (m *Token) XXX_Merge(src proto.Message) { + xxx_messageInfo_Token.Merge(m, src) +} +func (m *Token) XXX_Size() int { + return m.Size() +} +func (m *Token) XXX_DiscardUnknown() { + xxx_messageInfo_Token.DiscardUnknown(m) +} + +var xxx_messageInfo_Token proto.InternalMessageInfo + +func (m *Token) GetDenom() string { + if m != nil { + return m.Denom + } + return "" +} + +func (m *Token) GetAmount() string { + if m != nil { + return m.Amount + } + return "" +} + +func (m *Token) GetTrace() []string { + if m != nil { + return m.Trace + } + return nil +} + func init() { proto.RegisterType((*FungibleTokenPacketData)(nil), "ibc.applications.transfer.v2.FungibleTokenPacketData") + proto.RegisterType((*FungibleTokenPacketDataV2)(nil), "ibc.applications.transfer.v2.FungibleTokenPacketDataV2") + proto.RegisterType((*Token)(nil), "ibc.applications.transfer.v2.Token") } func init() { @@ -115,23 +257,29 @@ func init() { } var fileDescriptor_653ca2ce9a5ca313 = []byte{ - // 254 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x4c, 0x90, 0xb1, 0x4a, 0x34, 0x31, - 0x14, 0x46, 0x27, 0xff, 0xbf, 0xbb, 0x68, 0xca, 0x20, 0x3a, 0x88, 0x04, 0xb1, 0xd2, 0xc2, 0x09, - 0xac, 0x85, 0xd6, 0x22, 0xd6, 0x2a, 0x56, 0x76, 0x49, 0xe6, 0x3a, 0x86, 0x9d, 0xe4, 0x86, 0x24, - 0x33, 0xe0, 0x53, 0xe8, 0x63, 0x59, 0x6e, 0x69, 0x29, 0x33, 0x2f, 0x22, 0x9b, 0x51, 0xd9, 0x2e, - 0xe7, 0xe4, 0xbb, 0xcd, 0xa1, 0x67, 0x46, 0x69, 0x21, 0xbd, 0x6f, 0x8d, 0x96, 0xc9, 0xa0, 0x8b, - 0x22, 0x05, 0xe9, 0xe2, 0x33, 0x04, 0xd1, 0x2f, 0x85, 0x97, 0x7a, 0x05, 0xa9, 0xf2, 0x01, 0x13, - 0xb2, 0x23, 0xa3, 0x74, 0xb5, 0x3d, 0xad, 0x7e, 0xa7, 0x55, 0xbf, 0x3c, 0x79, 0x23, 0xf4, 0xe0, - 0xb6, 0x73, 0x8d, 0x51, 0x2d, 0x3c, 0xe2, 0x0a, 0xdc, 0x5d, 0xbe, 0xbd, 0x91, 0x49, 0xb2, 0x3d, - 0x3a, 0xaf, 0xc1, 0xa1, 0x2d, 0xc9, 0x31, 0x39, 0xdd, 0x7d, 0x98, 0x80, 0xed, 0xd3, 0x85, 0xb4, - 0xd8, 0xb9, 0x54, 0xfe, 0xcb, 0xfa, 0x87, 0x36, 0x3e, 0x82, 0xab, 0x21, 0x94, 0xff, 0x27, 0x3f, - 0x11, 0x3b, 0xa4, 0x3b, 0x01, 0x34, 0x98, 0x1e, 0x42, 0x39, 0xcb, 0x3f, 0x7f, 0xcc, 0x18, 0x9d, - 0x59, 0xb0, 0x58, 0xce, 0xb3, 0xcf, 0xef, 0xeb, 0xfb, 0x8f, 0x81, 0x93, 0xf5, 0xc0, 0xc9, 0xd7, - 0xc0, 0xc9, 0xfb, 0xc8, 0x8b, 0xf5, 0xc8, 0x8b, 0xcf, 0x91, 0x17, 0x4f, 0x97, 0x8d, 0x49, 0x2f, - 0x9d, 0xaa, 0x34, 0x5a, 0xa1, 0x31, 0x5a, 0x8c, 0xc2, 0x28, 0x7d, 0xde, 0xa0, 0xe8, 0xaf, 0x84, - 0xc5, 0xba, 0x6b, 0x21, 0x6e, 0xa2, 0x6c, 0xc5, 0x48, 0xaf, 0x1e, 0xa2, 0x5a, 0xe4, 0x12, 0x17, - 0xdf, 0x01, 0x00, 0x00, 0xff, 0xff, 0x93, 0x3d, 0xc6, 0x36, 0x36, 0x01, 0x00, 0x00, + // 344 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x94, 0x92, 0xb1, 0x4e, 0xfb, 0x30, + 0x10, 0xc6, 0xe3, 0x26, 0xa9, 0xfe, 0x7f, 0xb3, 0x45, 0x15, 0x84, 0x0a, 0x85, 0xaa, 0x2c, 0x65, + 0x20, 0x96, 0xc2, 0x00, 0x2b, 0x15, 0x62, 0x61, 0x81, 0x0a, 0x31, 0xb0, 0x39, 0xee, 0x11, 0xac, + 0x36, 0xb9, 0x28, 0x76, 0x22, 0xf1, 0x14, 0xf0, 0x14, 0x3c, 0x4b, 0xc7, 0x8e, 0x4c, 0x08, 0xb5, + 0x2f, 0x82, 0xe2, 0x14, 0x94, 0xa5, 0x48, 0x6c, 0xf7, 0x7d, 0xfe, 0xee, 0xf4, 0xb3, 0x7d, 0xf4, + 0x58, 0xc6, 0x82, 0xf1, 0x3c, 0x9f, 0x4b, 0xc1, 0xb5, 0xc4, 0x4c, 0x31, 0x5d, 0xf0, 0x4c, 0x3d, + 0x42, 0xc1, 0xaa, 0x88, 0xe5, 0x5c, 0xcc, 0x40, 0x87, 0x79, 0x81, 0x1a, 0xbd, 0x03, 0x19, 0x8b, + 0xb0, 0x1d, 0x0d, 0xbf, 0xa3, 0x61, 0x15, 0xf5, 0x7b, 0x09, 0x26, 0x68, 0x82, 0xac, 0xae, 0x9a, + 0x9e, 0xe1, 0x0b, 0xa1, 0x7b, 0x57, 0x65, 0x96, 0xc8, 0x78, 0x0e, 0x77, 0x38, 0x83, 0xec, 0xc6, + 0x4c, 0xbc, 0xe4, 0x9a, 0x7b, 0x3d, 0xea, 0x4e, 0x21, 0xc3, 0xd4, 0x27, 0x03, 0x32, 0xfa, 0x3f, + 0x69, 0x84, 0xb7, 0x4b, 0xbb, 0x3c, 0xc5, 0x32, 0xd3, 0x7e, 0xc7, 0xd8, 0x1b, 0x55, 0xfb, 0x0a, + 0xb2, 0x29, 0x14, 0xbe, 0xdd, 0xf8, 0x8d, 0xf2, 0xfa, 0xf4, 0x5f, 0x01, 0x02, 0x64, 0x05, 0x85, + 0xef, 0x98, 0x93, 0x1f, 0xed, 0x79, 0xd4, 0x49, 0x21, 0x45, 0xdf, 0x35, 0xbe, 0xa9, 0x87, 0x6f, + 0x84, 0xee, 0x6f, 0x21, 0xba, 0x8f, 0xbc, 0x0b, 0xda, 0xd5, 0xb5, 0xa9, 0x7c, 0x32, 0xb0, 0x47, + 0x3b, 0xd1, 0x51, 0xf8, 0xdb, 0xa5, 0x43, 0x33, 0x60, 0xec, 0x2c, 0x3e, 0x0e, 0xad, 0xc9, 0xa6, + 0xb1, 0x05, 0xda, 0xd9, 0x0a, 0x6a, 0x6f, 0x01, 0x75, 0x5a, 0xa0, 0xd7, 0xd4, 0x35, 0xe3, 0xff, + 0xf8, 0x4e, 0x3d, 0xea, 0xea, 0x82, 0x0b, 0xf0, 0xed, 0x81, 0x5d, 0xa7, 0x8d, 0x18, 0xdf, 0x2e, + 0x56, 0x01, 0x59, 0xae, 0x02, 0xf2, 0xb9, 0x0a, 0xc8, 0xeb, 0x3a, 0xb0, 0x96, 0xeb, 0xc0, 0x7a, + 0x5f, 0x07, 0xd6, 0xc3, 0x59, 0x22, 0xf5, 0x53, 0x19, 0x87, 0x02, 0x53, 0x26, 0x50, 0xa5, 0xa8, + 0x98, 0x8c, 0xc5, 0x49, 0x82, 0xac, 0x3a, 0x67, 0x29, 0x4e, 0xcb, 0x39, 0xa8, 0x7a, 0x41, 0x5a, + 0x8b, 0xa1, 0x9f, 0x73, 0x50, 0x71, 0xd7, 0xfc, 0xf0, 0xe9, 0x57, 0x00, 0x00, 0x00, 0xff, 0xff, + 0xe1, 0x0d, 0x6c, 0x28, 0x42, 0x02, 0x00, 0x00, } func (m *FungibleTokenPacketData) Marshal() (dAtA []byte, err error) { @@ -192,6 +340,110 @@ func (m *FungibleTokenPacketData) MarshalToSizedBuffer(dAtA []byte) (int, error) return len(dAtA) - i, nil } +func (m *FungibleTokenPacketDataV2) Marshal() (dAtA []byte, err error) { + size := m.Size() + dAtA = make([]byte, size) + n, err := m.MarshalToSizedBuffer(dAtA[:size]) + if err != nil { + return nil, err + } + return dAtA[:n], nil +} + +func (m *FungibleTokenPacketDataV2) MarshalTo(dAtA []byte) (int, error) { + size := m.Size() + return m.MarshalToSizedBuffer(dAtA[:size]) +} + +func (m *FungibleTokenPacketDataV2) MarshalToSizedBuffer(dAtA []byte) (int, error) { + i := len(dAtA) + _ = i + var l int + _ = l + if len(m.Memo) > 0 { + i -= len(m.Memo) + copy(dAtA[i:], m.Memo) + i = encodeVarintPacket(dAtA, i, uint64(len(m.Memo))) + i-- + dAtA[i] = 0x22 + } + if len(m.Receiver) > 0 { + i -= len(m.Receiver) + copy(dAtA[i:], m.Receiver) + i = encodeVarintPacket(dAtA, i, uint64(len(m.Receiver))) + i-- + dAtA[i] = 0x1a + } + if len(m.Sender) > 0 { + i -= len(m.Sender) + copy(dAtA[i:], m.Sender) + i = encodeVarintPacket(dAtA, i, uint64(len(m.Sender))) + i-- + dAtA[i] = 0x12 + } + if len(m.Tokens) > 0 { + for iNdEx := len(m.Tokens) - 1; iNdEx >= 0; iNdEx-- { + { + size, err := m.Tokens[iNdEx].MarshalToSizedBuffer(dAtA[:i]) + if err != nil { + return 0, err + } + i -= size + i = encodeVarintPacket(dAtA, i, uint64(size)) + } + i-- + dAtA[i] = 0xa + } + } + return len(dAtA) - i, nil +} + +func (m *Token) Marshal() (dAtA []byte, err error) { + size := m.Size() + dAtA = make([]byte, size) + n, err := m.MarshalToSizedBuffer(dAtA[:size]) + if err != nil { + return nil, err + } + return dAtA[:n], nil +} + +func (m *Token) MarshalTo(dAtA []byte) (int, error) { + size := m.Size() + return m.MarshalToSizedBuffer(dAtA[:size]) +} + +func (m *Token) MarshalToSizedBuffer(dAtA []byte) (int, error) { + i := len(dAtA) + _ = i + var l int + _ = l + if len(m.Trace) > 0 { + for iNdEx := len(m.Trace) - 1; iNdEx >= 0; iNdEx-- { + i -= len(m.Trace[iNdEx]) + copy(dAtA[i:], m.Trace[iNdEx]) + i = encodeVarintPacket(dAtA, i, uint64(len(m.Trace[iNdEx]))) + i-- + dAtA[i] = 0x1a + } + } + if len(m.Amount) > 0 { + i -= len(m.Amount) + copy(dAtA[i:], m.Amount) + i = encodeVarintPacket(dAtA, i, uint64(len(m.Amount))) + i-- + dAtA[i] = 0x12 + } + if len(m.Denom) > 0 { + i -= len(m.Denom) + copy(dAtA[i:], m.Denom) + i = encodeVarintPacket(dAtA, i, uint64(len(m.Denom))) + i-- + dAtA[i] = 0xa + } + return len(dAtA) - i, nil +} + func encodeVarintPacket(dAtA []byte, offset int, v uint64) int { offset -= sovPacket(v) base := offset @@ -232,6 +484,56 @@ func (m *FungibleTokenPacketData) Size() (n int) { return n } +func (m *FungibleTokenPacketDataV2) Size() (n int) { + if m == nil { + return 0 + } + var l int + _ = l + if len(m.Tokens) > 0 { + for _, e := range m.Tokens { + l = e.Size() + n += 1 + l + sovPacket(uint64(l)) + } + } + l = len(m.Sender) + if l > 0 { + n += 1 + l + sovPacket(uint64(l)) + } + l = len(m.Receiver) + if l > 0 { + n += 1 + l + sovPacket(uint64(l)) + } + l = len(m.Memo) + if l > 0 { + n += 1 + l + sovPacket(uint64(l)) + } + return n +} + +func (m *Token) Size() (n int) { + if m == nil { + return 0 + } + var l int + _ = l + l = len(m.Denom) + if l > 0 { + n += 1 + l + sovPacket(uint64(l)) + } + l = len(m.Amount) + if l > 0 { + n += 1 + l + sovPacket(uint64(l)) + } + if len(m.Trace) > 0 { + for _, s := range m.Trace { + l = len(s) + n += 1 + l + sovPacket(uint64(l)) + } + } + return n +} + func sovPacket(x uint64) (n int) { return (math_bits.Len64(x|1) + 6) / 7 } @@ -448,6 +750,332 @@ func (m *FungibleTokenPacketData) Unmarshal(dAtA []byte) error { } return nil } +func (m *FungibleTokenPacketDataV2) Unmarshal(dAtA []byte) error { + l := len(dAtA) + iNdEx := 0 + for iNdEx < l { + preIndex := iNdEx + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + fieldNum := int32(wire >> 3) + wireType := int(wire & 0x7) + if wireType == 4 { + return fmt.Errorf("proto: FungibleTokenPacketDataV2: wiretype end group for non-group") + } + if fieldNum <= 0 { + return fmt.Errorf("proto: FungibleTokenPacketDataV2: illegal tag %d (wire type %d)", fieldNum, wire) + } + switch fieldNum { + case 1: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Tokens", wireType) + } + var msglen int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + msglen |= int(b&0x7F) << shift + if b < 0x80 { + break + } + } + if msglen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + msglen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Tokens = append(m.Tokens, Token{}) + if err := m.Tokens[len(m.Tokens)-1].Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err + } + iNdEx = postIndex + case 2: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Sender", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Sender = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + case 3: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Receiver", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Receiver = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + case 4: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Memo", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Memo = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + default: + iNdEx = preIndex + skippy, err := skipPacket(dAtA[iNdEx:]) + if err != nil { + return err + } + if (skippy < 0) || (iNdEx+skippy) < 0 { + return ErrInvalidLengthPacket + } + if (iNdEx + skippy) > l { + return io.ErrUnexpectedEOF + } + iNdEx += skippy + } + } + + if iNdEx > l { + return io.ErrUnexpectedEOF + } + return nil +} +func (m *Token) Unmarshal(dAtA []byte) error { + l := len(dAtA) + iNdEx := 0 + for iNdEx < l { + preIndex := iNdEx + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + fieldNum := int32(wire >> 3) + wireType := int(wire & 0x7) + if wireType == 4 { + return fmt.Errorf("proto: Token: wiretype end group for non-group") + } + if fieldNum <= 0 { + return fmt.Errorf("proto: Token: illegal tag %d (wire type %d)", fieldNum, wire) + } + switch fieldNum { + case 1: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Denom", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Denom = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + case 2: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Amount", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Amount = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + case 3: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Trace", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Trace = append(m.Trace, string(dAtA[iNdEx:postIndex])) + iNdEx = postIndex + default: + iNdEx = preIndex + skippy, err := skipPacket(dAtA[iNdEx:]) + if err != nil { + return err + } + if (skippy < 0) || (iNdEx+skippy) < 0 { + return ErrInvalidLengthPacket + } + if (iNdEx + skippy) > l { + return io.ErrUnexpectedEOF + } + iNdEx += skippy + } + } + + if iNdEx > l { + return io.ErrUnexpectedEOF + } + return nil +} func skipPacket(dAtA []byte) (n int, err error) { l := len(dAtA) iNdEx := 0 diff --git a/modules/apps/transfer/types/packet_test.go b/modules/apps/transfer/types/packet_test.go index d06d0774c7d..71a0cfc81c7 100644 --- a/modules/apps/transfer/types/packet_test.go +++ b/modules/apps/transfer/types/packet_test.go @@ -8,6 +8,8 @@ import ( "github.com/stretchr/testify/require" "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" + ibctesting "github.com/cosmos/ibc-go/v8/testing" ) const ( @@ -159,3 +161,427 @@ func (suite *TypesTestSuite) TestFungibleTokenPacketDataOmitEmpty() { // check that the memo field is present in the marshalled bytes suite.Require().Contains(string(bz), "memo") } + +// TestFungibleTokenPacketDataValidateBasic tests ValidateBasic for FungibleTokenPacketData +func TestFungibleTokenPacketDataV2ValidateBasic(t *testing.T) { + testCases := []struct { + name string + packetData types.FungibleTokenPacketDataV2 + expErr error + }{ + { + "success: valid packet", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + nil, + }, + { + "success: valid packet with memo", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "memo", + ), + nil, + }, + { + "success: valid packet with large amount", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: largeAmount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "memo", + ), + nil, + }, + { + "failure: invalid denom", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: "", + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + types.ErrInvalidDenomForTransfer, + }, + { + "failure: invalid empty amount", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: "", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + types.ErrInvalidAmount, + }, + { + "failure: invalid empty token array", + types.NewFungibleTokenPacketDataV2( + []types.Token{}, + sender, + receiver, + "", + ), + types.ErrInvalidAmount, + }, + { + "failure: invalid zero amount", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: "0", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + types.ErrInvalidAmount, + }, + { + "failure: invalid negative amount", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: "-100", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + types.ErrInvalidAmount, + }, + { + "failure: invalid large amount", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: invalidLargeAmount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "memo", + ), + types.ErrInvalidAmount, + }, + { + "failure: missing sender address", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + "", + receiver, + "memo", + ), + ibcerrors.ErrInvalidAddress, + }, + { + "failure: missing recipient address", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + "", + "", + ), + ibcerrors.ErrInvalidAddress, + }, + { + "failure: memo field too large", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: largeAmount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + ibctesting.GenerateString(types.MaximumMemoLength+1), + ), + types.ErrInvalidMemo, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.packetData.ValidateBasic() + + expPass := tc.expErr == nil + if expPass { + require.NoError(t, err, tc.name) + } else { + require.ErrorContains(t, err, tc.expErr.Error(), tc.name) + } + }) + } +} + +func TestGetPacketSender(t *testing.T) { + testCases := []struct { + name string + packetData types.FungibleTokenPacketDataV2 + expSender string + }{ + { + "non-empty sender field", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + sender, + }, + { + "empty sender field", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + "", + receiver, + "abc", + ), + "", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expSender, tc.packetData.GetPacketSender(types.PortID)) + }) + } +} + +func TestPacketDataProvider(t *testing.T) { + testCases := []struct { + name string + packetData types.FungibleTokenPacketDataV2 + expCustomData interface{} + }{ + { + "success: src_callback key in memo", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + fmt.Sprintf(`{"src_callback": {"address": "%s"}}`, receiver)), + + map[string]interface{}{ + "address": receiver, + }, + }, + { + "success: src_callback key in memo with additional fields", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + fmt.Sprintf(`{"src_callback": {"address": "%s", "gas_limit": "200000"}}`, receiver)), + map[string]interface{}{ + "address": receiver, + "gas_limit": "200000", + }, + }, + { + "success: src_callback has string value", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + `{"src_callback": "string"}`), + "string", + }, + { + "failure: src_callback key not found memo", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + fmt.Sprintf(`{"dest_callback": {"address": "%s", "min_gas": "200000"}}`, receiver)), + nil, + }, + { + "failure: empty memo", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + ""), + nil, + }, + { + "failure: non-json memo", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "invalid"), + nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + customData := tc.packetData.GetCustomPacketData("src_callback") + require.Equal(t, tc.expCustomData, customData) + }) + } +} + +func TestFungibleTokenPacketDataOmitEmpty(t *testing.T) { + testCases := []struct { + name string + packetData types.FungibleTokenPacketDataV2 + expMemo bool + }{ + { + "empty memo field, resulting marshalled bytes should not contain the memo field", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + false, + }, + { + "non-empty memo field, resulting marshalled bytes should contain the memo field", + types.NewFungibleTokenPacketDataV2( + []types.Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "abc", + ), + true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + bz, err := json.Marshal(tc.packetData) + if tc.expMemo { + require.NoError(t, err, tc.name) + // check that the memo field is present in the marshalled bytes + require.Contains(t, string(bz), "memo") + } else { + require.NoError(t, err, tc.name) + // check that the memo field is not present in the marshalled bytes + require.NotContains(t, string(bz), "memo") + } + }) + } +} diff --git a/modules/apps/transfer/types/token.go b/modules/apps/transfer/types/token.go new file mode 100644 index 00000000000..68fd3383b75 --- /dev/null +++ b/modules/apps/transfer/types/token.go @@ -0,0 +1,68 @@ +package types + +import ( + "strings" + + errorsmod "cosmossdk.io/errors" + sdkmath "cosmossdk.io/math" +) + +// Validate validates a token denomination and trace identifiers. +func (t Token) Validate() error { + if strings.TrimSpace(t.Denom) == "" { + return errorsmod.Wrap(ErrInvalidDenomForTransfer, "denom cannot be empty") + } + + amount, ok := sdkmath.NewIntFromString(t.Amount) + if !ok { + return errorsmod.Wrapf(ErrInvalidAmount, "unable to parse transfer amount (%s) into math.Int", t.Amount) + } + + if !amount.IsPositive() { + return errorsmod.Wrapf(ErrInvalidAmount, "amount must be strictly positive: got %d", amount) + } + + if len(t.Trace) != 0 { + trace := strings.Join(t.Trace, "/") + identifiers := strings.Split(trace, "/") + + if err := validateTraceIdentifiers(identifiers); err != nil { + return err + } + } + + return nil +} + +// GetFullDenomPath returns the full denomination according to the ICS20 specification: +// tracePath + "/" + baseDenom +// If there exists no trace then the base denomination is returned. +func (t Token) GetFullDenomPath() string { + if len(t.Trace) == 0 { + return t.Denom + } + + return strings.Join(append(t.Trace, t.Denom), "/") +} + +// Tokens is a set of Token +type Tokens []Token + +// String prints out the tokens array as a string. +// If the array is empty, an empty string is returned +func (tokens Tokens) String() string { + if len(tokens) == 0 { + return "" + } else if len(tokens) == 1 { + return tokens[0].String() + } + + var out strings.Builder + for _, token := range tokens[:len(tokens)-1] { + out.WriteString(token.String()) // nolint:revive // no error returned by WriteString + out.WriteByte(',') //nolint:revive // no error returned by WriteByte + + } + out.WriteString(tokens[len(tokens)-1].String()) //nolint:revive + return out.String() +} diff --git a/modules/apps/transfer/types/token_test.go b/modules/apps/transfer/types/token_test.go new file mode 100644 index 00000000000..c80fb975ec9 --- /dev/null +++ b/modules/apps/transfer/types/token_test.go @@ -0,0 +1,253 @@ +package types + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" + sdk "github.com/cosmos/cosmos-sdk/types" +) + +const ( + denom = "atom/pool" + amount = "100" +) + +var ( + sender = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() + receiver = sdk.AccAddress("testaddr2").String() +) + +func TestGetFullDenomPath(t *testing.T) { + testCases := []struct { + name string + packetData FungibleTokenPacketDataV2 + expPath string + }{ + { + "denom path with trace", + NewFungibleTokenPacketDataV2( + []Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + }, + sender, + receiver, + "", + ), + "transfer/channel-0/transfer/channel-1/atom/pool", + }, + { + "nil trace", + NewFungibleTokenPacketDataV2( + []Token{ + { + Denom: denom, + Amount: amount, + Trace: []string{}, + }, + }, + sender, + receiver, + "", + ), + denom, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + path := tc.packetData.Tokens[0].GetFullDenomPath() + require.Equal(t, tc.expPath, path) + }) + } +} + +func TestValidate(t *testing.T) { + testCases := []struct { + name string + token Token + expError error + }{ + { + "success: multiple port channel pair denom", + Token{ + Denom: "atom", + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + nil, + }, + { + "success: one port channel pair denom", + Token{ + Denom: "uatom", + Amount: amount, + Trace: []string{"transfer/channel-1"}, + }, + nil, + }, + { + "success: non transfer port trace", + Token{ + Denom: "uatom", + Amount: amount, + Trace: []string{"transfer/channel-0", "transfer/channel-1", "transfer-custom/channel-2"}, + }, + nil, + }, + { + "failure: empty denom", + Token{ + Denom: "", + Amount: amount, + Trace: nil, + }, + ErrInvalidDenomForTransfer, + }, + { + "failure: invalid amount string", + Token{ + Denom: "atom", + Amount: "value", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + ErrInvalidAmount, + }, + { + "failure: amount is zero", + Token{ + Denom: "atom", + Amount: "0", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + ErrInvalidAmount, + }, + { + "failure: amount is negative", + Token{ + Denom: "atom", + Amount: "-1", + Trace: []string{"transfer/channel-0", "transfer/channel-1"}, + }, + ErrInvalidAmount, + }, + { + "failure: invalid identifier in trace", + Token{ + Denom: "uatom", + Amount: amount, + Trace: []string{"transfer/channel-1", "randomport"}, + }, + fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: [transfer channel-1 randomport]"), + }, + { + "failure: empty identifier in trace", + Token{ + Denom: "uatom", + Amount: amount, + Trace: []string{""}, + }, + fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: "), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.token.Validate() + expPass := tc.expError == nil + if expPass { + require.NoError(t, err, tc.name) + } else { + require.ErrorContains(t, err, tc.expError.Error(), tc.name) + } + }) + } +} + +func TestTokens_String(t *testing.T) { + cases := []struct { + name string + input Tokens + expected string + }{ + { + "empty tokens", + Tokens{}, + "", + }, + { + "single token, no trace", + Tokens{ + Token{ + Denom: "tree", + Amount: "1", + Trace: []string{}, + }, + }, + `denom:"tree" amount:"1" `, + }, + { + "single token with trace", + Tokens{ + Token{ + Denom: "tree", + Amount: "1", + Trace: []string{"portid/channelid"}, + }, + }, + `denom:"tree" amount:"1" trace:"portid/channelid" `, + }, + { + "multiple tokens, no trace", + Tokens{ + Token{ + Denom: "tree", + Amount: "1", + Trace: []string{}, + }, + Token{ + Denom: "gas", + Amount: "2", + Trace: []string{}, + }, + Token{ + Denom: "mineral", + Amount: "3", + Trace: []string{}, + }, + }, + `denom:"tree" amount:"1" ,denom:"gas" amount:"2" ,denom:"mineral" amount:"3" `, + }, + { + "multiple tokens, trace and no trace", + Tokens{ + Token{ + Denom: "tree", + Amount: "1", + Trace: []string{}, + }, + Token{ + Denom: "gas", + Amount: "2", + Trace: []string{"portid/channelid"}, + }, + Token{ + Denom: "mineral", + Amount: "3", + Trace: []string{"portid/channelid", "transfer/channel-52"}, + }, + }, + `denom:"tree" amount:"1" ,denom:"gas" amount:"2" trace:"portid/channelid" ,denom:"mineral" amount:"3" trace:"portid/channelid" trace:"transfer/channel-52" `, + }, + } + + for _, tt := range cases { + require.Equal(t, tt.expected, tt.input.String()) + } +} diff --git a/modules/apps/transfer/types/trace.go b/modules/apps/transfer/types/trace.go index 93aa7ee14bf..3287ffc1796 100644 --- a/modules/apps/transfer/types/trace.go +++ b/modules/apps/transfer/types/trace.go @@ -38,9 +38,9 @@ func ParseDenomTrace(rawDenom string) DenomTrace { } } - path, baseDenom := extractPathAndBaseFromFullDenom(denomSplit) + pathSlice, baseDenom := extractPathAndBaseFromFullDenom(denomSplit) return DenomTrace{ - Path: path, + Path: strings.Join(pathSlice, "/"), BaseDenom: baseDenom, } } @@ -84,7 +84,7 @@ func (dt DenomTrace) IsNativeDenom() bool { // extractPathAndBaseFromFullDenom returns the trace path and the base denom from // the elements that constitute the complete denom. -func extractPathAndBaseFromFullDenom(fullDenomItems []string) (string, string) { +func extractPathAndBaseFromFullDenom(fullDenomItems []string) ([]string, string) { var ( pathSlice []string baseDenomSlice []string @@ -109,12 +109,12 @@ func extractPathAndBaseFromFullDenom(fullDenomItems []string) (string, string) { } } - path := strings.Join(pathSlice, "/") baseDenom := strings.Join(baseDenomSlice, "/") - return path, baseDenom + return pathSlice, baseDenom } +// validateTraceIdentifiers validates the correctness of the trace associated with a particular base denom. func validateTraceIdentifiers(identifiers []string) error { if len(identifiers) == 0 || len(identifiers)%2 != 0 { return fmt.Errorf("trace info must come in pairs of port and channel identifiers '{portID}/{channelID}', got the identifiers: %s", identifiers) @@ -199,11 +199,12 @@ func ValidatePrefixedDenom(denom string) error { return nil } - if strings.TrimSpace(denomSplit[len(denomSplit)-1]) == "" { + pathSlice, baseDenom := extractPathAndBaseFromFullDenom(denomSplit) + if strings.TrimSpace(baseDenom) == "" { return errorsmod.Wrap(ErrInvalidDenomForTransfer, "base denomination cannot be blank") } - path, _ := extractPathAndBaseFromFullDenom(denomSplit) + path := strings.Join(pathSlice, "/") if path == "" { // NOTE: base denom contains slashes, so no base denomination validation return nil @@ -213,11 +214,11 @@ func ValidatePrefixedDenom(denom string) error { return validateTraceIdentifiers(identifiers) } -// ValidateIBCDenom validates that the given denomination is either: +// validateIBCDenom validates that the given denomination is either: // // - A valid base denomination (eg: 'uatom' or 'gamm/pool/1' as in https://github.com/cosmos/ibc-go/issues/894) // - A valid fungible token representation (i.e 'ibc/{hash}') per ADR 001 https://github.com/cosmos/ibc-go/blob/main/docs/architecture/adr-001-coin-source-tracing.md -func ValidateIBCDenom(denom string) error { +func validateIBCDenom(denom string) error { if err := sdk.ValidateDenom(denom); err != nil { return err } diff --git a/modules/apps/transfer/types/trace_test.go b/modules/apps/transfer/types/trace_test.go index 49623ed1dec..f7634a7a3b0 100644 --- a/modules/apps/transfer/types/trace_test.go +++ b/modules/apps/transfer/types/trace_test.go @@ -132,15 +132,16 @@ func TestValidatePrefixedDenom(t *testing.T) { expError bool }{ {"prefixed denom", "transfer/channel-1/uatom", false}, + {"prefixed denom with base denom with leading slash", "transfer/channel-1/uatom/", false}, {"prefixed denom with '/'", "transfer/channel-1/gamm/pool/1", false}, {"empty prefix", "/uatom", false}, {"empty identifiers", "//uatom", false}, {"base denom", "uatom", false}, {"base denom with single '/'", "erc20/0x85bcBCd7e79Ec36f4fBBDc54F90C643d921151AA", false}, {"base denom with multiple '/'s", "gamm/pool/1", false}, + {"single trace identifier", "transfer/", false}, {"invalid port ID", "(transfer)/channel-1/uatom", true}, {"empty denom", "", true}, - {"single trace identifier", "transfer/", true}, } for _, tc := range testCases { diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index b5e4bbe23bc..333519a86a2 100644 --- a/modules/apps/transfer/types/transfer_authorization.go +++ b/modules/apps/transfer/types/transfer_authorization.go @@ -21,6 +21,10 @@ import ( var _ authz.Authorization = (*TransferAuthorization)(nil) +const ( + allocationNotFound = -1 +) + // maxUint256 is the maximum value for a 256 bit unsigned integer. var maxUint256 = new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 256), big.NewInt(1)) @@ -37,45 +41,47 @@ func (TransferAuthorization) MsgTypeURL() string { } // Accept implements Authorization.Accept. -func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (authz.AcceptResponse, error) { +func (a TransferAuthorization) Accept(goCtx context.Context, msg proto.Message) (authz.AcceptResponse, error) { msgTransfer, ok := msg.(*MsgTransfer) if !ok { return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidType, "type mismatch") } - for index, allocation := range a.Allocations { - if !(allocation.SourceChannel == msgTransfer.SourceChannel && allocation.SourcePort == msgTransfer.SourcePort) { - continue - } + index := getAllocationIndex(*msgTransfer, a.Allocations) + if index == allocationNotFound { + return authz.AcceptResponse{}, errorsmod.Wrapf(ibcerrors.ErrNotFound, "requested port and channel allocation does not exist") + } - if !isAllowedAddress(sdk.UnwrapSDKContext(ctx), msgTransfer.Receiver, allocation.AllowList) { - return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "not allowed receiver address for transfer") - } + allocation := a.Allocations[index] + ctx := sdk.UnwrapSDKContext(goCtx) - err := validateMemo(sdk.UnwrapSDKContext(ctx), msgTransfer.Memo, allocation.AllowedPacketData) - if err != nil { - return authz.AcceptResponse{}, err - } + if !isAllowedAddress(ctx, msgTransfer.Receiver, allocation.AllowList) { + return authz.AcceptResponse{}, errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "not allowed receiver address for transfer") + } + + err := validateMemo(ctx, msgTransfer.Memo, allocation.AllowedPacketData) + if err != nil { + return authz.AcceptResponse{}, err + } + // bool flag to see if we have updated any of the allocations + allocationModified := false + + // update spend limit for each token in the MsgTransfer + for _, coin := range msgTransfer.GetCoins() { // If the spend limit is set to the MaxUint256 sentinel value, do not subtract the amount from the spend limit. - if allocation.SpendLimit.AmountOf(msgTransfer.Token.Denom).Equal(UnboundedSpendLimit()) { - return authz.AcceptResponse{Accept: true, Delete: false, Updated: nil}, nil + // if there is no unlimited spend, then we need to subtract the amount from the spend limit to get the limit left + if allocation.SpendLimit.AmountOf(coin.Denom).Equal(UnboundedSpendLimit()) { + continue } - limitLeft, isNegative := allocation.SpendLimit.SafeSub(msgTransfer.Token) + limitLeft, isNegative := a.Allocations[index].SpendLimit.SafeSub(coin) if isNegative { - return authz.AcceptResponse{}, errorsmod.Wrapf(ibcerrors.ErrInsufficientFunds, "requested amount is more than spend limit") + return authz.AcceptResponse{}, errorsmod.Wrapf(ibcerrors.ErrInsufficientFunds, "requested amount of token %s is more than spend limit", coin.Denom) } - if limitLeft.IsZero() { - a.Allocations = append(a.Allocations[:index], a.Allocations[index+1:]...) - if len(a.Allocations) == 0 { - return authz.AcceptResponse{Accept: true, Delete: true}, nil - } - return authz.AcceptResponse{Accept: true, Delete: false, Updated: &TransferAuthorization{ - Allocations: a.Allocations, - }}, nil - } + allocationModified = true + a.Allocations[index] = Allocation{ SourcePort: allocation.SourcePort, SourceChannel: allocation.SourceChannel, @@ -83,13 +89,24 @@ func (a TransferAuthorization) Accept(ctx context.Context, msg proto.Message) (a AllowList: allocation.AllowList, AllowedPacketData: allocation.AllowedPacketData, } + } + + // if the spend limit is zero of the associated allocation then we delete it. + if a.Allocations[index].SpendLimit.IsZero() { + a.Allocations = append(a.Allocations[:index], a.Allocations[index+1:]...) + } + + if len(a.Allocations) == 0 { + return authz.AcceptResponse{Accept: true, Delete: true}, nil + } - return authz.AcceptResponse{Accept: true, Delete: false, Updated: &TransferAuthorization{ - Allocations: a.Allocations, - }}, nil + if !allocationModified { + return authz.AcceptResponse{Accept: true, Delete: false, Updated: nil}, nil } - return authz.AcceptResponse{}, errorsmod.Wrapf(ibcerrors.ErrNotFound, "requested port and channel allocation does not exist") + return authz.AcceptResponse{Accept: true, Delete: false, Updated: &TransferAuthorization{ + Allocations: a.Allocations, + }}, nil } // ValidateBasic implements Authorization.ValidateBasic. @@ -192,3 +209,13 @@ func validateMemo(ctx sdk.Context, memo string, allowedMemos []string) error { func UnboundedSpendLimit() sdkmath.Int { return sdkmath.NewIntFromBigInt(maxUint256) } + +// getAllocationIndex ranges through a set of allocations, and returns the index of the allocation if found. If not, returns -1. +func getAllocationIndex(msg MsgTransfer, allocations []Allocation) int { + for index, allocation := range allocations { + if allocation.SourceChannel == msg.SourceChannel && allocation.SourcePort == msg.SourcePort { + return index + } + } + return allocationNotFound +} diff --git a/modules/apps/transfer/types/transfer_authorization_test.go b/modules/apps/transfer/types/transfer_authorization_test.go index 8ef1b36bdfc..c02be46aa03 100644 --- a/modules/apps/transfer/types/transfer_authorization_test.go +++ b/modules/apps/transfer/types/transfer_authorization_test.go @@ -20,7 +20,7 @@ const ( func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { var ( - msgTransfer types.MsgTransfer + msgTransfer *types.MsgTransfer transferAuthz types.TransferAuthorization ) @@ -72,15 +72,32 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { }, }, { - "success: with multiple allocations", + "success: with multiple allocations and multidenom transfer", func() { - alloc := types.Allocation{ + coins := sdk.NewCoins( + ibctesting.TestCoin, + sdk.NewCoin("atom", sdkmath.NewInt(100)), + sdk.NewCoin("osmo", sdkmath.NewInt(100)), + ) + + allocation := types.Allocation{ SourcePort: ibctesting.MockPort, SourceChannel: "channel-9", - SpendLimit: ibctesting.TestCoins, + SpendLimit: coins, } - transferAuthz.Allocations = append(transferAuthz.Allocations, alloc) + transferAuthz.Allocations = append(transferAuthz.Allocations, allocation) + + msgTransfer = types.NewMsgTransfer( + ibctesting.MockPort, + "channel-9", + coins, + suite.chainA.SenderAccount.GetAddress().String(), + ibctesting.TestAccAddress, + suite.chainB.GetTimeoutHeight(), + 0, + "", + ) }, func(res authz.AcceptResponse, err error) { suite.Require().NoError(err) @@ -91,7 +108,7 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { updatedAuthz, ok := res.Updated.(*types.TransferAuthorization) suite.Require().True(ok) - // assert spent spendlimit is removed from the list + // assert spent spendlimits are removed from the list suite.Require().Len(updatedAuthz.Allocations, 1) }, }, @@ -252,18 +269,20 @@ func (suite *TypesTestSuite) TestTransferAuthorizationAccept() { }, } - msgTransfer = types.MsgTransfer{ - SourcePort: path.EndpointA.ChannelConfig.PortID, - SourceChannel: path.EndpointA.ChannelID, - Token: ibctesting.TestCoin, - Sender: suite.chainA.SenderAccount.GetAddress().String(), - Receiver: ibctesting.TestAccAddress, - TimeoutHeight: suite.chainB.GetTimeoutHeight(), - } + msgTransfer = types.NewMsgTransfer( + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + sdk.NewCoins(ibctesting.TestCoin), + suite.chainA.SenderAccount.GetAddress().String(), + ibctesting.TestAccAddress, + suite.chainB.GetTimeoutHeight(), + 0, + "", + ) tc.malleate() - res, err := transferAuthz.Accept(suite.chainA.GetContext(), &msgTransfer) + res, err := transferAuthz.Accept(suite.chainA.GetContext(), msgTransfer) tc.assertResult(res, err) }) } diff --git a/modules/apps/transfer/types/tx.pb.go b/modules/apps/transfer/types/tx.pb.go index 6dbba5651c9..860b32b2549 100644 --- a/modules/apps/transfer/types/tx.pb.go +++ b/modules/apps/transfer/types/tx.pb.go @@ -54,6 +54,8 @@ type MsgTransfer struct { TimeoutTimestamp uint64 `protobuf:"varint,7,opt,name=timeout_timestamp,json=timeoutTimestamp,proto3" json:"timeout_timestamp,omitempty"` // optional memo Memo string `protobuf:"bytes,8,opt,name=memo,proto3" json:"memo,omitempty"` + // tokens to be transferred + Tokens []types.Coin `protobuf:"bytes,9,rep,name=tokens,proto3" json:"tokens"` } func (m *MsgTransfer) Reset() { *m = MsgTransfer{} } @@ -221,46 +223,47 @@ func init() { } var fileDescriptor_7401ed9bed2f8e09 = []byte{ - // 613 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x94, 0x53, 0xcf, 0x4f, 0x13, 0x4f, - 0x14, 0xef, 0x7e, 0x29, 0xfd, 0xc2, 0x54, 0x40, 0x56, 0x03, 0xcb, 0xc6, 0x6c, 0x49, 0x23, 0x09, - 0x96, 0x30, 0x93, 0x62, 0x0c, 0xa6, 0xc7, 0x72, 0xf1, 0x20, 0x09, 0x36, 0x78, 0xf1, 0x42, 0x76, - 0xa7, 0xcf, 0xed, 0x84, 0xee, 0xcc, 0x3a, 0x33, 0x6d, 0xf4, 0x62, 0x88, 0x27, 0xe3, 0xc9, 0x3f, - 0xc1, 0xa3, 0x47, 0xfe, 0x0c, 0x8e, 0x1c, 0x3d, 0x19, 0x03, 0x07, 0x2e, 0xfe, 0x11, 0x66, 0x66, - 0xa7, 0x75, 0xf5, 0x50, 0xf5, 0xb2, 0xfb, 0x7e, 0x7c, 0xde, 0xaf, 0xcf, 0x9b, 0x87, 0xb6, 0x58, - 0x42, 0x49, 0x9c, 0xe7, 0x43, 0x46, 0x63, 0xcd, 0x04, 0x57, 0x44, 0xcb, 0x98, 0xab, 0x97, 0x20, - 0xc9, 0xb8, 0x4d, 0xf4, 0x6b, 0x9c, 0x4b, 0xa1, 0x85, 0x7f, 0x8f, 0x25, 0x14, 0x97, 0x61, 0x78, - 0x02, 0xc3, 0xe3, 0x76, 0xb8, 0x1a, 0x67, 0x8c, 0x0b, 0x62, 0xbf, 0x45, 0x40, 0x78, 0x37, 0x15, - 0xa9, 0xb0, 0x22, 0x31, 0x92, 0xb3, 0xae, 0x53, 0xa1, 0x32, 0xa1, 0x48, 0xa6, 0x52, 0x93, 0x3e, - 0x53, 0xa9, 0x73, 0x44, 0xce, 0x91, 0xc4, 0x0a, 0xc8, 0xb8, 0x9d, 0x80, 0x8e, 0xdb, 0x84, 0x0a, - 0xc6, 0x9d, 0xbf, 0x61, 0xda, 0xa4, 0x42, 0x02, 0xa1, 0x43, 0x06, 0x5c, 0x9b, 0xe8, 0x42, 0x72, - 0x80, 0x9d, 0xd9, 0x73, 0x4c, 0x9a, 0xb5, 0xe0, 0xe6, 0xd9, 0x1c, 0xaa, 0x1f, 0xaa, 0xf4, 0xd8, - 0x59, 0xfd, 0x06, 0xaa, 0x2b, 0x31, 0x92, 0x14, 0x4e, 0x72, 0x21, 0x75, 0xe0, 0x6d, 0x7a, 0xdb, - 0x8b, 0x3d, 0x54, 0x98, 0x8e, 0x84, 0xd4, 0xfe, 0x16, 0x5a, 0x76, 0x00, 0x3a, 0x88, 0x39, 0x87, - 0x61, 0xf0, 0x9f, 0xc5, 0x2c, 0x15, 0xd6, 0x83, 0xc2, 0xe8, 0x77, 0xd0, 0xbc, 0x16, 0xa7, 0xc0, - 0x83, 0xb9, 0x4d, 0x6f, 0xbb, 0xbe, 0xb7, 0x81, 0x8b, 0xa9, 0xb0, 0x99, 0x0a, 0xbb, 0xa9, 0xf0, - 0x81, 0x60, 0xbc, 0xbb, 0x78, 0xf1, 0xb5, 0x51, 0xf9, 0x7c, 0x73, 0xde, 0xf2, 0x7a, 0x45, 0x88, - 0xbf, 0x86, 0x6a, 0x0a, 0x78, 0x1f, 0x64, 0x50, 0xb5, 0xa9, 0x9d, 0xe6, 0x87, 0x68, 0x41, 0x02, - 0x05, 0x36, 0x06, 0x19, 0xcc, 0x5b, 0xcf, 0x54, 0xf7, 0x9f, 0xa2, 0x65, 0xcd, 0x32, 0x10, 0x23, - 0x7d, 0x32, 0x00, 0x96, 0x0e, 0x74, 0x50, 0xb3, 0x85, 0x43, 0x6c, 0xd6, 0x65, 0xe8, 0xc2, 0x8e, - 0xa4, 0x71, 0x1b, 0x3f, 0xb1, 0x88, 0x72, 0xe5, 0x25, 0x17, 0x5c, 0x78, 0xfc, 0x1d, 0xb4, 0x3a, - 0xc9, 0x66, 0xfe, 0x4a, 0xc7, 0x59, 0x1e, 0xfc, 0xbf, 0xe9, 0x6d, 0x57, 0x7b, 0xb7, 0x9d, 0xe3, - 0x78, 0x62, 0xf7, 0x7d, 0x54, 0xcd, 0x20, 0x13, 0xc1, 0x82, 0x6d, 0xc9, 0xca, 0x9d, 0xd6, 0xfb, - 0x4f, 0x8d, 0xca, 0xbb, 0x9b, 0xf3, 0x96, 0xeb, 0xfd, 0xc3, 0xcd, 0x79, 0x6b, 0xad, 0xa0, 0x60, - 0x57, 0xf5, 0x4f, 0x49, 0x89, 0xf2, 0xe6, 0x3e, 0xba, 0x53, 0x52, 0x7b, 0xa0, 0x72, 0xc1, 0x15, - 0x98, 0x69, 0x15, 0xbc, 0x1a, 0x01, 0xa7, 0x60, 0xd7, 0x50, 0xed, 0x4d, 0xf5, 0x4e, 0xd5, 0xa4, - 0x6f, 0xbe, 0x45, 0x2b, 0x87, 0x2a, 0x7d, 0x9e, 0xf7, 0x63, 0x0d, 0x47, 0xb1, 0x8c, 0x33, 0x65, - 0xa9, 0x63, 0x29, 0x07, 0xe9, 0x36, 0xe7, 0x34, 0xbf, 0x8b, 0x6a, 0xb9, 0x45, 0xd8, 0x6d, 0xd5, - 0xf7, 0xee, 0xe3, 0x59, 0xaf, 0x18, 0x17, 0xd9, 0xba, 0x55, 0x43, 0x50, 0xcf, 0x45, 0x76, 0x56, - 0x7e, 0xce, 0x64, 0x93, 0x36, 0x37, 0xd0, 0xfa, 0x6f, 0xf5, 0x27, 0xcd, 0xef, 0x7d, 0xf7, 0xd0, - 0xdc, 0xa1, 0x4a, 0xfd, 0x01, 0x5a, 0x98, 0x3e, 0xad, 0x07, 0xb3, 0x6b, 0x96, 0x38, 0x08, 0xdb, - 0x7f, 0x0d, 0x9d, 0xd2, 0xa5, 0xd1, 0xad, 0x5f, 0x98, 0xd8, 0xfd, 0x63, 0x8a, 0x32, 0x3c, 0x7c, - 0xf4, 0x4f, 0xf0, 0x49, 0xd5, 0x70, 0xfe, 0xcc, 0x3c, 0x9f, 0xee, 0xb3, 0x8b, 0xab, 0xc8, 0xbb, - 0xbc, 0x8a, 0xbc, 0x6f, 0x57, 0x91, 0xf7, 0xf1, 0x3a, 0xaa, 0x5c, 0x5e, 0x47, 0x95, 0x2f, 0xd7, - 0x51, 0xe5, 0xc5, 0x7e, 0xca, 0xf4, 0x60, 0x94, 0x60, 0x2a, 0x32, 0xe2, 0x0e, 0x9b, 0x25, 0x74, - 0x37, 0x15, 0x64, 0xfc, 0x98, 0x64, 0xa2, 0x3f, 0x1a, 0x82, 0x32, 0xc7, 0x5a, 0x3a, 0x52, 0xfd, - 0x26, 0x07, 0x95, 0xd4, 0xec, 0x7d, 0x3e, 0xfc, 0x11, 0x00, 0x00, 0xff, 0xff, 0x3b, 0xda, 0xd1, - 0xc3, 0x96, 0x04, 0x00, 0x00, + // 630 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x94, 0x54, 0xb1, 0x6e, 0x13, 0x4d, + 0x10, 0xf6, 0xfd, 0x76, 0xfc, 0x27, 0x6b, 0x92, 0x90, 0x05, 0x25, 0x17, 0x0b, 0x9d, 0x2d, 0x8b, + 0x48, 0xc1, 0x51, 0x76, 0xe5, 0x20, 0x14, 0x64, 0x51, 0x39, 0x0d, 0x05, 0x91, 0x82, 0x15, 0x1a, + 0x9a, 0xe8, 0x6e, 0x3d, 0x9c, 0x57, 0xf1, 0xed, 0x1e, 0xb7, 0x6b, 0x0b, 0x1a, 0x84, 0xa8, 0x10, + 0x15, 0x8f, 0x40, 0x49, 0x99, 0x27, 0xa0, 0x4e, 0x99, 0x92, 0x0a, 0xa1, 0xa4, 0x48, 0xc3, 0x43, + 0xa0, 0xdd, 0x5b, 0x9b, 0x83, 0x22, 0x84, 0xc6, 0xde, 0x99, 0xf9, 0xe6, 0x9b, 0xf9, 0x66, 0x3c, + 0x46, 0x1b, 0x3c, 0x62, 0x34, 0x4c, 0xd3, 0x11, 0x67, 0xa1, 0xe6, 0x52, 0x28, 0xaa, 0xb3, 0x50, + 0xa8, 0x17, 0x90, 0xd1, 0x49, 0x87, 0xea, 0x57, 0x24, 0xcd, 0xa4, 0x96, 0xf8, 0x0e, 0x8f, 0x18, + 0x29, 0xc2, 0xc8, 0x14, 0x46, 0x26, 0x9d, 0xfa, 0x4a, 0x98, 0x70, 0x21, 0xa9, 0xfd, 0xcc, 0x13, + 0xea, 0xb7, 0x63, 0x19, 0x4b, 0xfb, 0xa4, 0xe6, 0xe5, 0xbc, 0x6b, 0x4c, 0xaa, 0x44, 0x2a, 0x9a, + 0xa8, 0xd8, 0xd0, 0x27, 0x2a, 0x76, 0x81, 0xc0, 0x05, 0xa2, 0x50, 0x01, 0x9d, 0x74, 0x22, 0xd0, + 0x61, 0x87, 0x32, 0xc9, 0x85, 0x8b, 0x37, 0x4c, 0x9b, 0x4c, 0x66, 0x40, 0xd9, 0x88, 0x83, 0xd0, + 0x26, 0x3b, 0x7f, 0x39, 0xc0, 0xd6, 0xd5, 0x3a, 0xa6, 0xcd, 0x5a, 0x70, 0xeb, 0x4b, 0x19, 0xd5, + 0xf6, 0x55, 0x7c, 0xe8, 0xbc, 0xb8, 0x81, 0x6a, 0x4a, 0x8e, 0x33, 0x06, 0x47, 0xa9, 0xcc, 0xb4, + 0xef, 0x35, 0xbd, 0xcd, 0x85, 0x3e, 0xca, 0x5d, 0x07, 0x32, 0xd3, 0x78, 0x03, 0x2d, 0x39, 0x00, + 0x1b, 0x86, 0x42, 0xc0, 0xc8, 0xff, 0xcf, 0x62, 0x16, 0x73, 0xef, 0x5e, 0xee, 0xc4, 0x5d, 0x34, + 0xa7, 0xe5, 0x31, 0x08, 0xbf, 0xdc, 0xf4, 0x36, 0x6b, 0x3b, 0xeb, 0x24, 0x57, 0x45, 0x8c, 0x2a, + 0xe2, 0x54, 0x91, 0x3d, 0xc9, 0x45, 0x6f, 0xe1, 0xf4, 0x5b, 0xa3, 0xf4, 0xf9, 0xf2, 0xa4, 0xed, + 0xf5, 0xf3, 0x14, 0xbc, 0x8a, 0xaa, 0x0a, 0xc4, 0x00, 0x32, 0xbf, 0x62, 0xa9, 0x9d, 0x85, 0xeb, + 0x68, 0x3e, 0x03, 0x06, 0x7c, 0x02, 0x99, 0x3f, 0x67, 0x23, 0x33, 0x1b, 0x3f, 0x41, 0x4b, 0x9a, + 0x27, 0x20, 0xc7, 0xfa, 0x68, 0x08, 0x3c, 0x1e, 0x6a, 0xbf, 0x6a, 0x0b, 0xd7, 0x89, 0x59, 0x97, + 0x19, 0x17, 0x71, 0x43, 0x9a, 0x74, 0xc8, 0x63, 0x8b, 0x28, 0x56, 0x5e, 0x74, 0xc9, 0x79, 0x04, + 0x6f, 0xa1, 0x95, 0x29, 0x9b, 0xf9, 0x56, 0x3a, 0x4c, 0x52, 0xff, 0xff, 0xa6, 0xb7, 0x59, 0xe9, + 0xdf, 0x74, 0x81, 0xc3, 0xa9, 0x1f, 0x63, 0x54, 0x49, 0x20, 0x91, 0xfe, 0xbc, 0x6d, 0xc9, 0xbe, + 0xf1, 0x23, 0x54, 0xb5, 0x5a, 0x94, 0xbf, 0xd0, 0x2c, 0x5f, 0x5b, 0xbf, 0xcb, 0xe9, 0xb6, 0xdf, + 0x7f, 0x6a, 0x94, 0xde, 0x5d, 0x9e, 0xb4, 0x9d, 0xf2, 0x0f, 0x97, 0x27, 0xed, 0xd5, 0x9c, 0x60, + 0x5b, 0x0d, 0x8e, 0x69, 0x61, 0x61, 0xad, 0x5d, 0x74, 0xab, 0x60, 0xf6, 0x41, 0xa5, 0x52, 0x28, + 0x30, 0xb3, 0x52, 0xf0, 0x72, 0x0c, 0x82, 0x81, 0x5d, 0x62, 0xa5, 0x3f, 0xb3, 0xbb, 0x15, 0x43, + 0xdf, 0x7a, 0x83, 0x96, 0xf7, 0x55, 0xfc, 0x2c, 0x1d, 0x84, 0x1a, 0x0e, 0xc2, 0x2c, 0x4c, 0x94, + 0x1d, 0x3c, 0x8f, 0x05, 0x64, 0x6e, 0xef, 0xce, 0xc2, 0x3d, 0x54, 0x4d, 0x2d, 0xc2, 0xee, 0xba, + 0xb6, 0x73, 0x97, 0x5c, 0x75, 0x03, 0x24, 0x67, 0xeb, 0x55, 0x8c, 0xb0, 0xbe, 0xcb, 0xec, 0x2e, + 0xff, 0xd2, 0x64, 0x49, 0x5b, 0xeb, 0x68, 0xed, 0x8f, 0xfa, 0xd3, 0xe6, 0x77, 0x7e, 0x78, 0xa8, + 0xbc, 0xaf, 0x62, 0x3c, 0x44, 0xf3, 0xb3, 0x1f, 0xe6, 0xbd, 0xab, 0x6b, 0x16, 0x66, 0x50, 0xef, + 0x5c, 0x1b, 0x3a, 0x1b, 0x97, 0x46, 0x37, 0x7e, 0x9b, 0xc4, 0xf6, 0x5f, 0x29, 0x8a, 0xf0, 0xfa, + 0x83, 0x7f, 0x82, 0x4f, 0xab, 0xd6, 0xe7, 0xde, 0x9a, 0xb5, 0xf7, 0x9e, 0x9e, 0x9e, 0x07, 0xde, + 0xd9, 0x79, 0xe0, 0x7d, 0x3f, 0x0f, 0xbc, 0x8f, 0x17, 0x41, 0xe9, 0xec, 0x22, 0x28, 0x7d, 0xbd, + 0x08, 0x4a, 0xcf, 0x77, 0x63, 0xae, 0x87, 0xe3, 0x88, 0x30, 0x99, 0x50, 0xf7, 0xb7, 0xc0, 0x23, + 0xb6, 0x1d, 0x4b, 0x3a, 0x79, 0x48, 0x13, 0x39, 0x18, 0x8f, 0x40, 0x99, 0x53, 0x2f, 0x9c, 0xb8, + 0x7e, 0x9d, 0x82, 0x8a, 0xaa, 0xf6, 0xba, 0xef, 0xff, 0x0c, 0x00, 0x00, 0xff, 0xff, 0x29, 0xee, + 0x20, 0x91, 0xd4, 0x04, 0x00, 0x00, } // Reference imports to suppress errors if they are not otherwise used. @@ -403,6 +406,20 @@ func (m *MsgTransfer) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l + if len(m.Tokens) > 0 { + for iNdEx := len(m.Tokens) - 1; iNdEx >= 0; iNdEx-- { + { + size, err := m.Tokens[iNdEx].MarshalToSizedBuffer(dAtA[:i]) + if err != nil { + return 0, err + } + i -= size + i = encodeVarintTx(dAtA, i, uint64(size)) + } + i-- + dAtA[i] = 0x4a + } + } if len(m.Memo) > 0 { i -= len(m.Memo) copy(dAtA[i:], m.Memo) @@ -601,6 +618,12 @@ func (m *MsgTransfer) Size() (n int) { if l > 0 { n += 1 + l + sovTx(uint64(l)) } + if len(m.Tokens) > 0 { + for _, e := range m.Tokens { + l = e.Size() + n += 1 + l + sovTx(uint64(l)) + } + } return n } @@ -920,6 +943,40 @@ func (m *MsgTransfer) Unmarshal(dAtA []byte) error { } m.Memo = string(dAtA[iNdEx:postIndex]) iNdEx = postIndex + case 9: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Tokens", wireType) + } + var msglen int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTx + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + msglen |= int(b&0x7F) << shift + if b < 0x80 { + break + } + } + if msglen < 0 { + return ErrInvalidLengthTx + } + postIndex := iNdEx + msglen + if postIndex < 0 { + return ErrInvalidLengthTx + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Tokens = append(m.Tokens, types.Coin{}) + if err := m.Tokens[len(m.Tokens)-1].Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err + } + iNdEx = postIndex default: iNdEx = preIndex skippy, err := skipTx(dAtA[iNdEx:]) diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go index 23fc5705233..994a90fa17b 100644 --- a/modules/core/05-port/types/module.go +++ b/modules/core/05-port/types/module.go @@ -194,5 +194,5 @@ type Middleware interface { // request the packet data to be unmarshaled by the base application. type PacketDataUnmarshaler interface { // UnmarshalPacketData unmarshals the packet data into a concrete type - UnmarshalPacketData([]byte) (interface{}, error) + UnmarshalPacketData(ctx sdk.Context, portID, channelID string, bz []byte) (interface{}, error) } diff --git a/proto/ibc/applications/transfer/v1/tx.proto b/proto/ibc/applications/transfer/v1/tx.proto index 42c70d3bedc..52e5d29b7e1 100644 --- a/proto/ibc/applications/transfer/v1/tx.proto +++ b/proto/ibc/applications/transfer/v1/tx.proto @@ -49,6 +49,8 @@ message MsgTransfer { uint64 timeout_timestamp = 7; // optional memo string memo = 8; + // tokens to be transferred + repeated cosmos.base.v1beta1.Coin tokens = 9 [(gogoproto.nullable) = false, (amino.dont_omitempty) = true]; } // MsgTransferResponse defines the Msg/Transfer response type. diff --git a/proto/ibc/applications/transfer/v2/packet.proto b/proto/ibc/applications/transfer/v2/packet.proto index bff35bdd6d3..3614bcb85ed 100644 --- a/proto/ibc/applications/transfer/v2/packet.proto +++ b/proto/ibc/applications/transfer/v2/packet.proto @@ -4,6 +4,8 @@ package ibc.applications.transfer.v2; option go_package = "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"; +import "gogoproto/gogo.proto"; + // FungibleTokenPacketData defines a struct for the packet payload // See FungibleTokenPacketData spec: // https://github.com/cosmos/ibc/tree/master/spec/app/ics-020-fungible-token-transfer#data-structures @@ -19,3 +21,27 @@ message FungibleTokenPacketData { // optional memo string memo = 5; } + +// FungibleTokenPacketDataV2 defines a struct for the packet payload +// See FungibleTokenPacketDataV2 spec: +// https://github.com/cosmos/ibc/tree/master/spec/app/ics-020-fungible-token-transfer#data-structures +message FungibleTokenPacketDataV2 { + // the tokens to be transferred + repeated Token tokens = 1 [(gogoproto.nullable) = false]; + // the sender address + string sender = 2; + // the recipient address on the destination chain + string receiver = 3; + // optional memo + string memo = 4; +} + +// Token defines a struct which represents a token to be transferred. +message Token { + // the base token denomination to be transferred + string denom = 1; + // the token amount to be transferred + string amount = 2; + // the trace of the token + repeated string trace = 3; +} diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go index b3bd2432de3..963945f5330 100644 --- a/testing/mock/ibc_module.go +++ b/testing/mock/ibc_module.go @@ -219,7 +219,7 @@ func (im IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, // UnmarshalPacketData returns the MockPacketData. This function implements the optional // PacketDataUnmarshaler interface required for ADR 008 support. -func (IBCModule) UnmarshalPacketData(bz []byte) (interface{}, error) { +func (IBCModule) UnmarshalPacketData(_ sdk.Context, _, _ string, bz []byte) (interface{}, error) { if reflect.DeepEqual(bz, MockPacketData) { return MockPacketData, nil } diff --git a/testing/path.go b/testing/path.go index 7ee411ec88f..e877a517508 100644 --- a/testing/path.go +++ b/testing/path.go @@ -47,8 +47,8 @@ func NewTransferPath(chainA, chainB *TestChain) *Path { path := NewPath(chainA, chainB) path.EndpointA.ChannelConfig.PortID = TransferPort path.EndpointB.ChannelConfig.PortID = TransferPort - path.EndpointA.ChannelConfig.Version = transfertypes.Version - path.EndpointB.ChannelConfig.Version = transfertypes.Version + path.EndpointA.ChannelConfig.Version = transfertypes.V2 + path.EndpointB.ChannelConfig.Version = transfertypes.V2 return path } diff --git a/testing/solomachine.go b/testing/solomachine.go index 34877aa8535..2a10fc50107 100644 --- a/testing/solomachine.go +++ b/testing/solomachine.go @@ -312,7 +312,7 @@ func (solo *Solomachine) ConnOpenAck(chain *TestChain, clientID, connectionID st func (solo *Solomachine) ChanOpenInit(chain *TestChain, connectionID string) string { msgChanOpenInit := channeltypes.NewMsgChannelOpenInit( transfertypes.PortID, - transfertypes.Version, + transfertypes.V2, channeltypes.UNORDERED, []string{connectionID}, transfertypes.PortID, @@ -332,12 +332,12 @@ func (solo *Solomachine) ChanOpenInit(chain *TestChain, connectionID string) str // ChanOpenAck performs the channel open ack handshake step on the tendermint chain for the associated // solo machine client. func (solo *Solomachine) ChanOpenAck(chain *TestChain, channelID string) { - tryProof := solo.GenerateChanOpenTryProof(transfertypes.PortID, transfertypes.Version, channelID) + tryProof := solo.GenerateChanOpenTryProof(transfertypes.PortID, transfertypes.V2, channelID) msgChanOpenAck := channeltypes.NewMsgChannelOpenAck( transfertypes.PortID, channelID, channelIDSolomachine, - transfertypes.Version, + transfertypes.V2, tryProof, clienttypes.ZeroHeight(), chain.SenderAccount.GetAddress().String(), @@ -351,7 +351,7 @@ func (solo *Solomachine) ChanOpenAck(chain *TestChain, channelID string) { // ChanCloseConfirm performs the channel close confirm handshake step on the tendermint chain for the associated // solo machine client. func (solo *Solomachine) ChanCloseConfirm(chain *TestChain, portID, channelID string) { - initProof := solo.GenerateChanClosedProof(portID, transfertypes.Version, channelID) + initProof := solo.GenerateChanClosedProof(portID, transfertypes.V2, channelID) msgChanCloseConfirm := channeltypes.NewMsgChannelCloseConfirm( portID, channelID, @@ -369,21 +369,22 @@ func (solo *Solomachine) ChanCloseConfirm(chain *TestChain, portID, channelID st // SendTransfer constructs a MsgTransfer and sends the message to the given chain. Any number of optional // functions can be provided which will modify the MsgTransfer before SendMsgs is called. func (solo *Solomachine) SendTransfer(chain *TestChain, portID, channelID string, fns ...func(*transfertypes.MsgTransfer)) channeltypes.Packet { - msgTransfer := transfertypes.MsgTransfer{ - SourcePort: portID, - SourceChannel: channelID, - Token: sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100)), - Sender: chain.SenderAccount.GetAddress().String(), - Receiver: chain.SenderAccount.GetAddress().String(), - TimeoutHeight: clienttypes.ZeroHeight(), - TimeoutTimestamp: uint64(chain.GetContext().BlockTime().Add(time.Hour).UnixNano()), - } + msgTransfer := transfertypes.NewMsgTransfer( + portID, + channelID, + sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100))), + chain.SenderAccount.GetAddress().String(), + chain.SenderAccount.GetAddress().String(), + clienttypes.ZeroHeight(), + uint64(chain.GetContext().BlockTime().Add(time.Hour).UnixNano()), + "", + ) for _, fn := range fns { - fn(&msgTransfer) + fn(msgTransfer) } - res, err := chain.SendMsgs(&msgTransfer) + res, err := chain.SendMsgs(msgTransfer) require.NoError(solo.t, err) packet, err := ParsePacketFromEvents(res.Events) @@ -441,7 +442,7 @@ func (solo *Solomachine) TimeoutPacket(chain *TestChain, packet channeltypes.Pac // TimeoutPacketOnClose creates a channel closed and unreceived packet proof and broadcasts a MsgTimeoutOnClose. func (solo *Solomachine) TimeoutPacketOnClose(chain *TestChain, packet channeltypes.Packet, channelID string) { - closedProof := solo.GenerateChanClosedProof(transfertypes.PortID, transfertypes.Version, channelID) + closedProof := solo.GenerateChanClosedProof(transfertypes.PortID, transfertypes.V2, channelID) unreceivedProof := solo.GenerateReceiptAbsenceProof(packet) msgTimeout := channeltypes.NewMsgTimeoutOnClose( packet,