diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go index c87591e6c9f..f8435946c47 100644 --- a/modules/apps/transfer/ibc_module.go +++ b/modules/apps/transfer/ibc_module.go @@ -181,7 +181,7 @@ func (IBCModule) unmarshalPacketDataBytesToICS20V2(bz []byte, ics20Version strin 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-V2 transfer packet data: %s", err.Error()) + return types.FungibleTokenPacketDataV2{}, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS20-V1 transfer packet data: %s", err.Error()) } if err := datav1.ValidateBasic(); err != nil { @@ -240,26 +240,28 @@ func (im IBCModule) OnRecvPacket( } } - ctx.EventManager().EmitEvent(sdk.NewEvent( - types.EventTypePacket, + eventAttributes := []sdk.Attribute{ sdk.NewAttribute(types.AttributeKeySender, data.Sender), sdk.NewAttribute(types.AttributeKeyReceiver, data.Receiver), sdk.NewAttribute(types.AttributeKeyTokens, types.Tokens(data.Tokens).String()), sdk.NewAttribute(types.AttributeKeyMemo, data.Memo), - )) - - eventAttributes := []sdk.Attribute{ - sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), sdk.NewAttribute(types.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack.Success())), } + if ackErr != nil { eventAttributes = append(eventAttributes, sdk.NewAttribute(types.AttributeKeyAckError, ackErr.Error())) } - ctx.EventManager().EmitEvent(sdk.NewEvent( - sdk.EventTypeMessage, - eventAttributes..., - )) + 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 @@ -298,11 +300,11 @@ func (im IBCModule) OnAcknowledgementPacket( sdk.NewAttribute(types.AttributeKeyReceiver, data.Receiver), 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), - sdk.NewAttribute(types.AttributeKeyAck, ack.String()), ), }) @@ -350,9 +352,8 @@ func (im IBCModule) OnTimeoutPacket( ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypeTimeout, - sdk.NewAttribute(sdk.AttributeKeySender, data.Sender), - sdk.NewAttribute(types.AttributeKeyReceiver, data.Receiver), - sdk.NewAttribute(types.AttributeKeyTokens, types.Tokens(data.Tokens).String()), + sdk.NewAttribute(types.AttributeKeyReceiver, data.Sender), + sdk.NewAttribute(types.AttributeKeyRefundTokens, types.Tokens(data.Tokens).String()), sdk.NewAttribute(types.AttributeKeyMemo, data.Memo), ), sdk.NewEvent( diff --git a/modules/apps/transfer/keeper/msg_server.go b/modules/apps/transfer/keeper/msg_server.go index eaac2fb3ae9..f6a04ab4b81 100644 --- a/modules/apps/transfer/keeper/msg_server.go +++ b/modules/apps/transfer/keeper/msg_server.go @@ -38,10 +38,8 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types. return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "cannot transfer multiple coins with ics20-1") } - for _, coin := range coins { - if !k.bankKeeper.IsSendEnabledCoin(ctx, coin) { - return nil, errorsmod.Wrapf(types.ErrSendDisabled, "transfers are currently disabled for %s", coin.Denom) - } + if err := k.bankKeeper.IsSendEnabledCoins(ctx, coins...); err != nil { + return nil, errorsmod.Wrapf(types.ErrSendDisabled, err.Error()) } if k.bankKeeper.BlockedAddr(sender) { diff --git a/modules/apps/transfer/keeper/msg_server_test.go b/modules/apps/transfer/keeper/msg_server_test.go index e6d04001c4f..4806fc23f5d 100644 --- a/modules/apps/transfer/keeper/msg_server_test.go +++ b/modules/apps/transfer/keeper/msg_server_test.go @@ -113,6 +113,36 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { types.ErrSendDisabled, false, }, + { + "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() { diff --git a/modules/apps/transfer/types/events.go b/modules/apps/transfer/types/events.go index 7a4bfaf2a04..f310ba5d0d9 100644 --- a/modules/apps/transfer/types/events.go +++ b/modules/apps/transfer/types/events.go @@ -13,8 +13,7 @@ const ( AttributeKeyDenom = "denom" 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/msgs.go b/modules/apps/transfer/types/msgs.go index fd28b8778e5..3fdfcfefe85 100644 --- a/modules/apps/transfer/types/msgs.go +++ b/modules/apps/transfer/types/msgs.go @@ -126,7 +126,7 @@ func isValidIBCCoin(coin sdk.Coin) bool { return validateIBCCoin(coin) == nil } -// isValidIBCCoin returns true if the token provided is valid, +// 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 { diff --git a/modules/apps/transfer/types/token.go b/modules/apps/transfer/types/token.go index 4fe2928489d..68fd3383b75 100644 --- a/modules/apps/transfer/types/token.go +++ b/modules/apps/transfer/types/token.go @@ -22,14 +22,16 @@ func (t Token) Validate() error { return errorsmod.Wrapf(ErrInvalidAmount, "amount must be strictly positive: got %d", amount) } - if len(t.Trace) == 0 { - return nil - } + if len(t.Trace) != 0 { + trace := strings.Join(t.Trace, "/") + identifiers := strings.Split(trace, "/") - trace := strings.Join(t.Trace, "/") - identifiers := strings.Split(trace, "/") + if err := validateTraceIdentifiers(identifiers); err != nil { + return err + } + } - return validateTraceIdentifiers(identifiers) + return nil } // GetFullDenomPath returns the full denomination according to the ICS20 specification: diff --git a/modules/apps/transfer/types/token_test.go b/modules/apps/transfer/types/token_test.go index 7ae88757b90..c80fb975ec9 100644 --- a/modules/apps/transfer/types/token_test.go +++ b/modules/apps/transfer/types/token_test.go @@ -110,6 +110,33 @@ func TestValidate(t *testing.T) { }, 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{