refactor: make transfer module v2 use same core paths as v1 (#7754)

* reuse transfer keeper in v2

* remove keeper from transfer module v2

* moved some comments around and disabled forwarding

* code review fixes

* Update modules/apps/transfer/keeper/relay_test.go

---------

Co-authored-by: Aditya <[email protected]>

Author: gjermundgaraba

modules/apps/transfer/ibc_module.go

@@ -11,6 +11,7 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
+	"github.com/cosmos/ibc-go/v9/modules/apps/transfer/internal/telemetry"
 	"github.com/cosmos/ibc-go/v9/modules/apps/transfer/keeper"
 	"github.com/cosmos/ibc-go/v9/modules/apps/transfer/types"
 	channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
@@ -171,12 +172,11 @@ func (im IBCModule) OnRecvPacket(
 	relayer sdk.AccAddress,
 ) ibcexported.Acknowledgement {
 	var (
+		ack    ibcexported.Acknowledgement
 		ackErr error
 		data   types.FungibleTokenPacketDataV2
 	)
 
-	ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)})
-
 	// we are explicitly wrapping this emit event call in an anonymous function so that
 	// the packet data is evaluated after it has been assigned a value.
 	defer func() {
@@ -192,12 +192,36 @@ func (im IBCModule) OnRecvPacket(
 		return ack
 	}
 
-	if ackErr = im.keeper.OnRecvPacket(ctx, packet, data); ackErr != nil {
+	receivedCoins, ackErr := im.keeper.OnRecvPacket(
+		ctx,
+		data,
+		packet.SourcePort,
+		packet.SourceChannel,
+		packet.DestinationPort,
+		packet.DestinationChannel,
+	)
+	if ackErr != nil {
 		ack = channeltypes.NewErrorAcknowledgement(ackErr)
 		im.keeper.Logger.Error(fmt.Sprintf("%s sequence %d", ackErr.Error(), packet.Sequence))
 		return ack
 	}
 
+	if data.HasForwarding() {
+		// we are now sending from the forward escrow address to the final receiver address.
+		if ackErr = im.keeper.ForwardPacket(ctx, data, packet, receivedCoins); ackErr != nil {
+			ack = channeltypes.NewErrorAcknowledgement(ackErr)
+			im.keeper.Logger.Error(fmt.Sprintf("%s sequence %d", ackErr.Error(), packet.Sequence))
+			return ack
+
+		}
+
+		ack = nil
+	}
+
+	ack = channeltypes.NewResultAcknowledgement([]byte{byte(1)})
+
+	telemetry.ReportOnRecvPacket(packet, data.Tokens)
+
 	im.keeper.Logger.Info("successfully handled ICS-20 packet", "sequence", packet.Sequence)
 
 	if data.HasForwarding() {
@@ -227,10 +251,16 @@ func (im IBCModule) OnAcknowledgementPacket(
 		return err
 	}
 
-	if err := im.keeper.OnAcknowledgementPacket(ctx, packet, data, ack); err != nil {
+	if err := im.keeper.OnAcknowledgementPacket(ctx, packet.SourcePort, packet.SourceChannel, data, ack); err != nil {
 		return err
 	}
 
+	if forwardedPacket, isForwarded := im.keeper.GetForwardedPacket(ctx, packet.SourcePort, packet.SourceChannel, packet.Sequence); isForwarded {
+		if err := im.keeper.HandleForwardedPacketAcknowledgement(ctx, packet, forwardedPacket, data, ack); err != nil {
+			return err
+		}
+	}
+
 	return im.keeper.EmitOnAcknowledgementPacketEvent(ctx, data, ack)
 }
 
@@ -247,10 +277,16 @@ func (im IBCModule) OnTimeoutPacket(
 	}
 
 	// refund tokens
-	if err := im.keeper.OnTimeoutPacket(ctx, packet, data); err != nil {
+	if err := im.keeper.OnTimeoutPacket(ctx, packet.SourcePort, packet.SourceChannel, data); err != nil {
 		return err
 	}
 
+	if forwardedPacket, isForwarded := im.keeper.GetForwardedPacket(ctx, packet.SourcePort, packet.SourceChannel, packet.Sequence); isForwarded {
+		if err := im.keeper.HandleForwardedPacketTimeout(ctx, packet, forwardedPacket, data); err != nil {
+			return err
+		}
+	}
+
 	return im.keeper.EmitOnTimeoutEvent(ctx, data)
 }
 

modules/apps/transfer/keeper/events.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/cosmos/ibc-go/v9/modules/apps/transfer/types"
 	channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
+	ibcexported "github.com/cosmos/ibc-go/v9/modules/core/exported"
 )
 
 // EmitTransferEvent emits an ibc transfer event on successful transfers.
@@ -36,7 +37,7 @@ func (k Keeper) EmitTransferEvent(ctx context.Context, sender, receiver string,
 }
 
 // EmitOnRecvPacketEvent emits a fungible token packet event in the OnRecvPacket callback
-func (k Keeper) EmitOnRecvPacketEvent(ctx context.Context, packetData types.FungibleTokenPacketDataV2, ack channeltypes.Acknowledgement, ackErr error) error {
+func (k Keeper) EmitOnRecvPacketEvent(ctx context.Context, packetData types.FungibleTokenPacketDataV2, ack ibcexported.Acknowledgement, ackErr error) error {
 	tokensStr := mustMarshalJSON(packetData.Tokens)
 	forwardingHopStr := mustMarshalJSON(packetData.Forwarding.Hops)
 

modules/apps/transfer/keeper/export_test.go

@@ -39,11 +39,6 @@ func (k Keeper) UnwindHops(ctx sdk.Context, msg *types.MsgTransfer) (*types.MsgT
 	return k.unwindHops(ctx, msg)
 }
 
-// GetForwardedPacket is a wrapper around getForwardedPacket for testing purposes.
-func (k Keeper) GetForwardedPacket(ctx sdk.Context, portID, channelID string, sequence uint64) (channeltypes.Packet, bool) {
-	return k.getForwardedPacket(ctx, portID, channelID, sequence)
-}
-
 // SetForwardedPacket is a wrapper around setForwardedPacket for testing purposes.
 func (k Keeper) SetForwardedPacket(ctx sdk.Context, portID, channelID string, sequence uint64, packet channeltypes.Packet) {
 	k.setForwardedPacket(ctx, portID, channelID, sequence, packet)

modules/apps/transfer/keeper/forwarding.go

@@ -13,8 +13,8 @@ import (
 	ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
 )
 
-// forwardPacket forwards a fungible FungibleTokenPacketDataV2 to the next hop in the forwarding path.
-func (k Keeper) forwardPacket(ctx context.Context, data types.FungibleTokenPacketDataV2, packet channeltypes.Packet, receivedCoins sdk.Coins) error {
+// ForwardPacket forwards a fungible FungibleTokenPacketDataV2 to the next hop in the forwarding path.
+func (k Keeper) ForwardPacket(ctx context.Context, data types.FungibleTokenPacketDataV2, packet channeltypes.Packet, receivedCoins sdk.Coins) error {
 	var nextForwardingPath *types.Forwarding
 	if len(data.Forwarding.Hops) > 1 {
 		// remove the first hop since we are going to send to the first hop now and we want to propagate the rest of the hops to the receiver

modules/apps/transfer/keeper/keeper.go

@@ -308,7 +308,7 @@ func (k Keeper) setForwardedPacket(ctx context.Context, portID, channelID string
 }
 
 // getForwardedPacket gets the forwarded packet from the store.
-func (k Keeper) getForwardedPacket(ctx context.Context, portID, channelID string, sequence uint64) (channeltypes.Packet, bool) {
+func (k Keeper) GetForwardedPacket(ctx context.Context, portID, channelID string, sequence uint64) (channeltypes.Packet, bool) {
 	store := k.KVStoreService.OpenKVStore(ctx)
 	bz, err := store.Get(types.PacketForwardKey(portID, channelID, sequence))
 	if err != nil {

modules/apps/transfer/keeper/mbt_relay_test.go

@@ -365,20 +365,27 @@ func (suite *KeeperTestSuite) TestModelBasedRelay() {
 
 					}
 				case "OnRecvPacket":
-					err = suite.chainB.GetSimApp().TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, tc.packet.Data)
+					_, err = suite.chainB.GetSimApp().TransferKeeper.OnRecvPacket(
+						suite.chainB.GetContext(),
+						tc.packet.Data,
+						packet.SourcePort,
+						packet.SourceChannel,
+						packet.DestinationPort,
+						packet.DestinationChannel,
+					)
 
 				case "OnTimeoutPacket":
 					registerDenomFn()
-					err = suite.chainB.GetSimApp().TransferKeeper.OnTimeoutPacket(suite.chainB.GetContext(), packet, tc.packet.Data)
+					err = suite.chainB.GetSimApp().TransferKeeper.OnTimeoutPacket(suite.chainB.GetContext(), packet.SourcePort, packet.SourceChannel, tc.packet.Data)
 
 				case "OnRecvAcknowledgementResult":
 					err = suite.chainB.GetSimApp().TransferKeeper.OnAcknowledgementPacket(
-						suite.chainB.GetContext(), packet, tc.packet.Data,
+						suite.chainB.GetContext(), packet.SourcePort, packet.SourceChannel, tc.packet.Data,
 						channeltypes.NewResultAcknowledgement(nil))
 				case "OnRecvAcknowledgementError":
 					registerDenomFn()
 					err = suite.chainB.GetSimApp().TransferKeeper.OnAcknowledgementPacket(
-						suite.chainB.GetContext(), packet, tc.packet.Data,
+						suite.chainB.GetContext(), packet.SourcePort, packet.SourceChannel, tc.packet.Data,
 						channeltypes.NewErrorAcknowledgement(fmt.Errorf("MBT Error Acknowledgement")))
 				default:
 					err = fmt.Errorf("Unknown handler:  %s", tc.handler)

modules/apps/transfer/keeper/msg_server.go

@@ -8,7 +8,9 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
+	"github.com/cosmos/ibc-go/v9/modules/apps/transfer/internal/telemetry"
 	"github.com/cosmos/ibc-go/v9/modules/apps/transfer/types"
+	channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
 	ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
 )
 
@@ -25,30 +27,70 @@ func (k Keeper) Transfer(ctx context.Context, msg *types.MsgTransfer) (*types.Ms
 		return nil, err
 	}
 
-	coins := msg.GetCoins()
+	if msg.Forwarding.GetUnwind() {
+		msg, err = k.unwindHops(ctx, msg)
+		if err != nil {
+			return nil, err
+		}
+	}
 
-	if err := k.BankKeeper.IsSendEnabledCoins(ctx, coins...); err != nil {
-		return nil, errorsmod.Wrap(types.ErrSendDisabled, err.Error())
+	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)
 	}
 
-	if k.IsBlockedAddr(sender) {
-		return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "%s is not allowed to send funds", sender)
+	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)
 	}
 
-	if msg.Forwarding.GetUnwind() {
-		msg, err = k.unwindHops(ctx, msg)
+	coins := msg.GetCoins()
+	hops := msg.Forwarding.GetHops()
+	if appVersion == types.V1 {
+		// ics20-1 only supports a single coin, so if that is the current version, we must only process a single coin.
+		if len(coins) > 1 {
+			return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "cannot transfer multiple coins with %s", types.V1)
+		}
+
+		// ics20-1 does not support forwarding, so if that is the current version, we must reject the transfer.
+		if len(hops) > 0 {
+			return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "cannot forward coins with %s", types.V1)
+		}
+	}
+
+	tokens := make([]types.Token, len(coins))
+
+	for i, coin := range coins {
+		tokens[i], err = k.tokenFromCoin(ctx, coin)
 		if err != nil {
 			return nil, err
 		}
 	}
 
-	sequence, err := k.sendTransfer(
-		ctx, msg.SourcePort, msg.SourceChannel, coins, sender, msg.Receiver, msg.TimeoutHeight, msg.TimeoutTimestamp,
-		msg.Memo, msg.Forwarding.GetHops())
+	if err := k.SendTransfer(ctx, msg.SourcePort, msg.SourceChannel, tokens, sender); err != nil {
+		return nil, err
+	}
+
+	packetDataBytes, err := createPacketDataBytesFromVersion(
+		appVersion, sender.String(), msg.Receiver, msg.Memo, tokens, hops,
+	)
 	if err != nil {
 		return nil, err
 	}
 
+	sequence, err := k.ics4Wrapper.SendPacket(ctx, msg.SourcePort, msg.SourceChannel, msg.TimeoutHeight, msg.TimeoutTimestamp, packetDataBytes)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := k.EmitTransferEvent(ctx, sender.String(), msg.Receiver, tokens, msg.Memo, hops); err != nil {
+		return nil, err
+	}
+
+	destinationPort := channel.Counterparty.PortId
+	destinationChannel := channel.Counterparty.ChannelId
+	telemetry.ReportTransfer(msg.SourcePort, msg.SourceChannel, destinationPort, destinationChannel, tokens)
+
 	k.Logger.Info("IBC fungible token transfer", "tokens", coins, "sender", msg.Sender, "receiver", msg.Receiver)
 
 	return &types.MsgTransferResponse{Sequence: sequence}, nil