MsgTransfer For Eureka (#7957)

* initial progress * progress * initial build and existing tests pass * testing * documentation for tests * refactor * fix regression * Update modules/apps/transfer/types/packet.go Co-authored-by: Gjermund Garaba <[email protected]> * remove clientKeeperv2 necessity in code * remove clientkeeperv2 in transfer keeper --------- Co-authored-by: Gjermund Garaba <[email protected]> (cherry picked from commit 9f677ba)
cosmos · Feb 26, 2025 · f0c94fd · f0c94fd
1 parent 1f54e4f
commit f0c94fd
Show file tree

Hide file tree

Showing 19 changed files with 419 additions and 133 deletions.
diff --git a/modules/apps/callbacks/testing/simapp/app.go b/modules/apps/callbacks/testing/simapp/app.go
@@ -390,6 +390,7 @@ func NewSimApp(
 		appCodec, runtime.NewKVStoreService(keys[ibctransfertypes.StoreKey]), app.GetSubspace(ibctransfertypes.ModuleName),
 		app.IBCFeeKeeper, // ISC4 Wrapper: fee IBC middleware
 		app.IBCKeeper.ChannelKeeper,
+		app.IBCKeeper.ChannelKeeperV2,
 		app.AccountKeeper, app.BankKeeper,
 		authtypes.NewModuleAddress(govtypes.ModuleName).String(),
 	)

diff --git a/modules/apps/transfer/keeper/keeper.go b/modules/apps/transfer/keeper/keeper.go
@@ -29,10 +29,11 @@ type Keeper struct {
 	cdc            codec.BinaryCodec
 	legacySubspace types.ParamSubspace
 
-	ics4Wrapper   porttypes.ICS4Wrapper
-	channelKeeper types.ChannelKeeper
-	AuthKeeper    types.AccountKeeper
-	BankKeeper    types.BankKeeper
+	ics4Wrapper     porttypes.ICS4Wrapper
+	channelKeeper   types.ChannelKeeper
+	channelKeeperV2 types.ChannelKeeperV2
+	AuthKeeper      types.AccountKeeper
+	BankKeeper      types.BankKeeper
 
 	// the address capable of executing a MsgUpdateParams message. Typically, this
 	// should be the x/gov module account.
@@ -46,6 +47,7 @@ func NewKeeper(
 	legacySubspace types.ParamSubspace,
 	ics4Wrapper porttypes.ICS4Wrapper,
 	channelKeeper types.ChannelKeeper,
+	channelKeeperV2 types.ChannelKeeperV2,
 	authKeeper types.AccountKeeper,
 	bankKeeper types.BankKeeper,
 	authority string,
@@ -60,14 +62,15 @@ func NewKeeper(
 	}
 
 	return Keeper{
-		cdc:            cdc,
-		storeService:   storeService,
-		legacySubspace: legacySubspace,
-		ics4Wrapper:    ics4Wrapper,
-		channelKeeper:  channelKeeper,
-		AuthKeeper:     authKeeper,
-		BankKeeper:     bankKeeper,
-		authority:      authority,
+		cdc:             cdc,
+		storeService:    storeService,
+		legacySubspace:  legacySubspace,
+		ics4Wrapper:     ics4Wrapper,
+		channelKeeper:   channelKeeper,
+		channelKeeperV2: channelKeeperV2,
+		AuthKeeper:      authKeeper,
+		BankKeeper:      bankKeeper,
+		authority:       authority,
 	}
 }
 

diff --git a/modules/apps/transfer/keeper/keeper_test.go b/modules/apps/transfer/keeper/keeper_test.go
@@ -60,6 +60,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
 				suite.chainA.GetSimApp().GetSubspace(types.ModuleName),
 				suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
 				suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
+				suite.chainA.GetSimApp().IBCKeeper.ChannelKeeperV2,
 				suite.chainA.GetSimApp().AccountKeeper,
 				suite.chainA.GetSimApp().BankKeeper,
 				suite.chainA.GetSimApp().ICAControllerKeeper.GetAuthority(),
@@ -72,6 +73,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
 				suite.chainA.GetSimApp().GetSubspace(types.ModuleName),
 				suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
 				suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
+				suite.chainA.GetSimApp().IBCKeeper.ChannelKeeperV2,
 				authkeeper.AccountKeeper{}, // empty account keeper
 				suite.chainA.GetSimApp().BankKeeper,
 				suite.chainA.GetSimApp().ICAControllerKeeper.GetAuthority(),
@@ -84,6 +86,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
 				suite.chainA.GetSimApp().GetSubspace(types.ModuleName),
 				suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
 				suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper,
+				suite.chainA.GetSimApp().IBCKeeper.ChannelKeeperV2,
 				suite.chainA.GetSimApp().AccountKeeper,
 				suite.chainA.GetSimApp().BankKeeper,
 				"", // authority

diff --git a/modules/apps/transfer/keeper/msg_server.go b/modules/apps/transfer/keeper/msg_server.go
@@ -10,7 +10,8 @@ import (
 	"github.com/cosmos/ibc-go/v10/modules/apps/transfer/internal/events"
 	"github.com/cosmos/ibc-go/v10/modules/apps/transfer/internal/telemetry"
 	"github.com/cosmos/ibc-go/v10/modules/apps/transfer/types"
-	channeltypes "github.com/cosmos/ibc-go/v10/modules/core/04-channel/types"
+	clienttypes "github.com/cosmos/ibc-go/v10/modules/core/02-client/types"
+	channeltypesv2 "github.com/cosmos/ibc-go/v10/modules/core/04-channel/v2/types"
 	ibcerrors "github.com/cosmos/ibc-go/v10/modules/core/errors"
 )
 
@@ -29,16 +30,6 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
 		return nil, err
 	}
 
-	channel, found := k.channelKeeper.GetChannel(ctx, msg.SourcePort, msg.SourceChannel)
-	if !found {
-		return nil, errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", msg.SourcePort, msg.SourceChannel)
-	}
-
-	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)
-	}
-
 	coin := msg.Token
 
 	// Using types.UnboundedSpendLimit allows us to send the entire balance of a given denom.
@@ -54,31 +45,76 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
 		return nil, err
 	}
 
-	if err := k.SendTransfer(ctx, msg.SourcePort, msg.SourceChannel, token, sender); err != nil {
-		return nil, err
+	packetData := types.NewFungibleTokenPacketData(token.Denom.Path(), token.Amount, sender.String(), msg.Receiver, msg.Memo)
+
+	if err := packetData.ValidateBasic(); err != nil {
+		return nil, errorsmod.Wrapf(err, "failed to validate %s packet data", types.V1)
 	}
 
-	packetDataBytes, err := createPacketDataBytesFromVersion(
-		appVersion, sender.String(), msg.Receiver, msg.Memo, token,
-	)
+	// if a channel exists with source channel, then use IBC V1 protocol
+	// otherwise use IBC V2 protocol
+	channel, isIBCV1 := k.channelKeeper.GetChannel(ctx, msg.SourcePort, msg.SourceChannel)
+
+	var sequence uint64
+	if isIBCV1 {
+		// if a V1 channel exists for the source channel, then use IBC V1 protocol
+		sequence, err = k.transferV1Packet(ctx, msg.SourceChannel, token, msg.TimeoutHeight, msg.TimeoutTimestamp, packetData)
+		// telemetry for transfer occurs here, in IBC V2 this is done in the onSendPacket callback
+		telemetry.ReportTransfer(msg.SourcePort, msg.SourceChannel, channel.Counterparty.PortId, channel.Counterparty.ChannelId, token)
+	} else {
+		// otherwise try to send an IBC V2 packet, if the sourceChannel is not a IBC V2 client
+		// then core IBC will return a CounterpartyNotFound error
+		sequence, err = k.transferV2Packet(ctx, msg.Encoding, msg.SourceChannel, msg.TimeoutTimestamp, packetData)
+	}
 	if err != nil {
 		return nil, err
 	}
 
-	sequence, err := k.ics4Wrapper.SendPacket(ctx, msg.SourcePort, msg.SourceChannel, msg.TimeoutHeight, msg.TimeoutTimestamp, packetDataBytes)
+	k.Logger(ctx).Info("IBC fungible token transfer", "token", coin, "sender", msg.Sender, "receiver", msg.Receiver)
+
+	return &types.MsgTransferResponse{Sequence: sequence}, nil
+}
+
+func (k Keeper) transferV1Packet(ctx sdk.Context, sourceChannel string, token types.Token, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, packetData types.FungibleTokenPacketData) (uint64, error) {
+	if err := k.SendTransfer(ctx, types.PortID, sourceChannel, token, sdk.MustAccAddressFromBech32(packetData.Sender)); err != nil {
+		return 0, err
+	}
+
+	packetDataBytes := packetData.GetBytes()
+	sequence, err := k.ics4Wrapper.SendPacket(ctx, types.PortID, sourceChannel, timeoutHeight, timeoutTimestamp, packetDataBytes)
 	if err != nil {
-		return nil, err
+		return 0, err
 	}
 
-	events.EmitTransferEvent(ctx, sender.String(), msg.Receiver, token, msg.Memo)
+	events.EmitTransferEvent(ctx, packetData.Sender, packetData.Receiver, token, packetData.Memo)
 
-	destinationPort := channel.Counterparty.PortId
-	destinationChannel := channel.Counterparty.ChannelId
-	telemetry.ReportTransfer(msg.SourcePort, msg.SourceChannel, destinationPort, destinationChannel, token)
+	return sequence, nil
+}
 
-	k.Logger(ctx).Info("IBC fungible token transfer", "token", coin, "sender", msg.Sender, "receiver", msg.Receiver)
+func (k Keeper) transferV2Packet(ctx sdk.Context, encoding, sourceChannel string, timeoutTimestamp uint64, packetData types.FungibleTokenPacketData) (uint64, error) {
+	if encoding == "" {
+		encoding = types.EncodingJSON
+	}
 
-	return &types.MsgTransferResponse{Sequence: sequence}, nil
+	data, err := types.MarshalPacketData(packetData, types.V1, encoding)
+	if err != nil {
+		return 0, err
+	}
+
+	payload := channeltypesv2.NewPayload(
+		types.PortID, types.PortID,
+		types.V1, encoding, data,
+	)
+	msg := channeltypesv2.NewMsgSendPacket(
+		sourceChannel, timeoutTimestamp,
+		packetData.Sender, payload,
+	)
+
+	res, err := k.channelKeeperV2.SendPacket(ctx, msg)
+	if err != nil {
+		return 0, err
+	}
+	return res.Sequence, nil
 }
 
 // UpdateParams defines an rpc handler method for MsgUpdateParams. Updates the ibc-transfer module's parameters.

diff --git a/modules/apps/transfer/keeper/msg_server_test.go b/modules/apps/transfer/keeper/msg_server_test.go
@@ -3,6 +3,7 @@ package keeper_test
 import (
 	"errors"
 	"strings"
+	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
@@ -11,7 +12,8 @@ import (
 	abci "github.com/cometbft/cometbft/abci/types"
 
 	"github.com/cosmos/ibc-go/v10/modules/apps/transfer/types"
-	channeltypes "github.com/cosmos/ibc-go/v10/modules/core/04-channel/types"
+	clienttypes "github.com/cosmos/ibc-go/v10/modules/core/02-client/types"
+	clienttypesv2 "github.com/cosmos/ibc-go/v10/modules/core/02-client/v2/types"
 	ibcerrors "github.com/cosmos/ibc-go/v10/modules/core/errors"
 	ibctesting "github.com/cosmos/ibc-go/v10/testing"
 )
@@ -90,7 +92,7 @@ func (suite *KeeperTestSuite) TestMsgTransfer() {
 			func() {
 				msg.SourceChannel = "channel-100"
 			},
-			channeltypes.ErrChannelNotFound,
+			clienttypesv2.ErrCounterpartyNotFound,
 		},
 	}
 
@@ -109,7 +111,7 @@ func (suite *KeeperTestSuite) TestMsgTransfer() {
 				ibctesting.TestCoin,
 				suite.chainA.SenderAccount.GetAddress().String(),
 				suite.chainB.SenderAccount.GetAddress().String(),
-				suite.chainB.GetTimeoutHeight(), 0, // only use timeout height
+				clienttypes.Height{}, suite.chainB.GetTimeoutTimestamp(), // only use timeout height
 				"memo",
 			)
 
@@ -160,6 +162,151 @@ func (suite *KeeperTestSuite) TestMsgTransfer() {
 	}
 }
 
+// TestMsgTransfer tests Transfer rpc handler with IBC V2 protocol
+func (suite *KeeperTestSuite) TestMsgTransferIBCV2() {
+	var msg *types.MsgTransfer
+	var path *ibctesting.Path
+
+	testCases := []struct {
+		name     string
+		malleate func()
+		expError error
+	}{
+		{
+			"success",
+			func() {
+				msg.Token = ibctesting.TestCoin
+			},
+			nil,
+		},
+		{
+			"bank send enabled for denoms",
+			func() {
+				err := suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(),
+					banktypes.Params{
+						SendEnabled: []*banktypes.SendEnabled{
+							{Denom: sdk.DefaultBondDenom, Enabled: true},
+							{Denom: ibctesting.SecondaryDenom, Enabled: true},
+						},
+					},
+				)
+				suite.Require().NoError(err)
+			},
+			nil,
+		},
+		{
+			"failure: send transfers disabled",
+			func() {
+				suite.chainA.GetSimApp().TransferKeeper.SetParams(suite.chainA.GetContext(),
+					types.Params{
+						SendEnabled: false,
+					},
+				)
+			},
+			types.ErrSendDisabled,
+		},
+		{
+			"failure: invalid sender",
+			func() {
+				msg.Sender = "address"
+			},
+			errors.New("decoding bech32 failed"),
+		},
+		{
+			"failure: sender is a blocked address",
+			func() {
+				msg.Sender = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(minttypes.ModuleName).String()
+			},
+			ibcerrors.ErrUnauthorized,
+		},
+		{
+			"failure: bank send disabled",
+			func() {
+				err := suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(),
+					banktypes.Params{
+						SendEnabled: []*banktypes.SendEnabled{{Denom: sdk.DefaultBondDenom, Enabled: false}},
+					},
+				)
+				suite.Require().NoError(err)
+			},
+			types.ErrSendDisabled,
+		},
+		{
+			"failure: client does not exist",
+			func() {
+				msg.SourceChannel = "07-tendermint-500"
+			},
+			clienttypesv2.ErrCounterpartyNotFound,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+
+		suite.Run(tc.name, func() {
+			suite.SetupTest()
+
+			path = ibctesting.NewPath(suite.chainA, suite.chainB)
+			path.SetupV2()
+
+			timeoutTimestamp := uint64(suite.chainA.GetContext().BlockTime().Add(time.Hour).Unix())
+
+			msg = types.NewMsgTransfer(
+				types.PortID,
+				path.EndpointA.ClientID, // use eureka client id
+				ibctesting.TestCoin,
+				suite.chainA.SenderAccount.GetAddress().String(),
+				suite.chainB.SenderAccount.GetAddress().String(),
+				clienttypes.Height{}, timeoutTimestamp, // only use timeout timestamp
+				"memo",
+			)
+
+			// send some coins of the second denom from bank module to the sender account as well
+			err := suite.chainA.GetSimApp().BankKeeper.MintCoins(suite.chainA.GetContext(), types.ModuleName, sdk.NewCoins(ibctesting.SecondaryTestCoin))
+			suite.Require().NoError(err)
+			err = suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, suite.chainA.SenderAccount.GetAddress(), sdk.NewCoins(ibctesting.SecondaryTestCoin))
+			suite.Require().NoError(err)
+
+			tc.malleate()
+
+			ctx := suite.chainA.GetContext()
+
+			token, err := suite.chainA.GetSimApp().TransferKeeper.TokenFromCoin(ctx, msg.Token)
+			suite.Require().NoError(err)
+
+			res, err := suite.chainA.GetSimApp().TransferKeeper.Transfer(ctx, msg)
+
+			// Verify events
+			var expEvents []abci.Event
+			events := ctx.EventManager().Events().ToABCIEvents()
+
+			expEvents = sdk.Events{
+				sdk.NewEvent(types.EventTypeTransfer,
+					sdk.NewAttribute(types.AttributeKeySender, msg.Sender),
+					sdk.NewAttribute(types.AttributeKeyReceiver, msg.Receiver),
+					sdk.NewAttribute(types.AttributeKeyDenom, token.Denom.Path()),
+					sdk.NewAttribute(types.AttributeKeyAmount, msg.Token.Amount.String()),
+					sdk.NewAttribute(types.AttributeKeyMemo, msg.Memo),
+				),
+				sdk.NewEvent(
+					sdk.EventTypeMessage,
+					sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
+				),
+			}.ToABCIEvents()
+
+			if tc.expError == nil {
+				suite.Require().NoError(err)
+				suite.Require().NotNil(res)
+				suite.Require().NotEqual(res.Sequence, uint64(0))
+				ibctesting.AssertEvents(&suite.Suite, expEvents, events)
+			} else {
+				suite.Require().Nil(res)
+				suite.Require().True(errors.Is(err, tc.expError) || strings.Contains(err.Error(), tc.expError.Error()), err.Error())
+			}
+		})
+	}
+}
+
 // TestUpdateParams tests UpdateParams rpc handler
 func (suite *KeeperTestSuite) TestUpdateParams() {
 	signer := suite.chainA.GetSimApp().TransferKeeper.GetAuthority()

diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go
@@ -147,14 +147,6 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
 			},
 			types.ErrInvalidAmount,
 		},
-		{
-			"failure: source channel not found",
-			func() {
-				// channel references wrong ID
-				path.EndpointA.ChannelID = ibctesting.InvalidID
-			},
-			channeltypes.ErrChannelNotFound,
-		},
 		{
 			"failure: sender account is blocked",
 			func() {