feat(transfer): add unwinding ability (#6656)

* Create ontimeoutpacket test for forwarding * Propagate ack on A * Refactoring * Minor changes * Added comments * Fix type name. * Gofumpt * Update modules/apps/transfer/keeper/relay_forwarding_test.go Co-authored-by: Carlos Rodriguez <[email protected]> * Update modules/apps/transfer/keeper/relay_forwarding_test.go Co-authored-by: Carlos Rodriguez <[email protected]> * Update modules/apps/transfer/keeper/relay_forwarding_test.go Co-authored-by: Carlos Rodriguez <[email protected]> * Update modules/apps/transfer/keeper/relay_forwarding_test.go Co-authored-by: Carlos Rodriguez <[email protected]> * Add godoc to test. * Changed trace construction * Update modules/apps/transfer/keeper/relay_forwarding_test.go Co-authored-by: Carlos Rodriguez <[email protected]> * remove error msg parameter from helper function * Add test for forwarded packet * Construct packet for B ack check. * PR feedback * feat(transfer): add unwind, refactor proto structure. gen-all * tests(transfer/types): fix test failures in types tests. * tests(transfer/keeper): fix test failures in keeper tests. * cli(transfer): fix cli usage. pending flag for unwind. * tests(callbacks): fix failing tests in callbacks. * tests(transfer/internal): fix failures in internal package. * tests(transfer): fix test failures in top level tranfer package. * tests(ica/host/keeper): fix repr of msg transfer in ica host msg execution. * lint(all): lint this bad boy * chore(transfer/types): amend validation for MsgTransfer's ShouldBeForwarded, add tests for ForwardedPacketData, minor nits. * nit(self): only pass relevant fields to create packet data; minor comment improvement. * Apply suggestions from code review Co-authored-by: Carlos Rodriguez <[email protected]> * chore(merge): fix merge issues. * chore(proto): mention optional nature of fields. * memo: do not drop it * validation: drop requirement on memo being empty on msg transfer. * feat(transfer): add unwinding ability, wip. * Added unwind to allocation forwarding. * Add tests and move some validation * Missing import * Fixed validation and added test * PR Feedback * Return nil when returning an error. * Cleaner comment * Add test case for multiple hos --------- Co-authored-by: bznein <[email protected]> Co-authored-by: Nikolas De Giorgis <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>
cosmos · Jun 25, 2024 · 59e3df7 · 59e3df7
1 parent 2b4d24b
commit 59e3df7
Show file tree

Hide file tree

Showing 9 changed files with 526 additions and 73 deletions.
diff --git a/modules/apps/transfer/keeper/export_test.go b/modules/apps/transfer/keeper/export_test.go
@@ -33,6 +33,11 @@ func (k Keeper) TokenFromCoin(ctx sdk.Context, coin sdk.Coin) (types.Token, erro
 	return k.tokenFromCoin(ctx, coin)
 }
 
+// UnwindHops is a wrapper around unwindToken for testing purposes.
+func (k Keeper) UnwindHops(ctx sdk.Context, msg *types.MsgTransfer) (*types.MsgTransfer, error) {
+	return k.unwindHops(ctx, msg)
+}
+
 // CreatePacketDataBytesFromVersion is a wrapper around createPacketDataBytesFromVersion for testing purposes
 func CreatePacketDataBytesFromVersion(appVersion, sender, receiver, memo string, tokens types.Tokens, hops []types.Hop) []byte {
 	return createPacketDataBytesFromVersion(appVersion, sender, receiver, memo, tokens, hops)

diff --git a/modules/apps/transfer/keeper/msg_server.go b/modules/apps/transfer/keeper/msg_server.go
@@ -36,6 +36,13 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
 		return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "%s is not allowed to send funds", sender)
 	}
 
+	if msg.Forwarding.Unwind {
+		msg, err = k.unwindHops(ctx, msg)
+		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)
@@ -59,3 +66,32 @@ func (k Keeper) UpdateParams(goCtx context.Context, msg *types.MsgUpdateParams)
 
 	return &types.MsgUpdateParamsResponse{}, nil
 }
+
+// unwindHops unwinds the hops present in the tokens denomination and returns the message modified to reflect
+// the unwound path to take. It assumes that only a single token is present (as this is verified in ValidateBasic)
+// in the tokens list and ensures that the token is not native to the chain.
+func (k Keeper) unwindHops(ctx sdk.Context, msg *types.MsgTransfer) (*types.MsgTransfer, error) {
+	coins := msg.GetCoins()
+	token, err := k.tokenFromCoin(ctx, coins[0])
+	if err != nil {
+		return nil, err
+	}
+
+	if token.Denom.IsNative() {
+		return nil, errorsmod.Wrap(types.ErrInvalidForwarding, "cannot unwind a native token")
+	}
+	var unwindHops []types.Hop
+	for _, trace := range token.Denom.Trace[1:] {
+		unwindHops = append(unwindHops, types.Hop{PortId: trace.PortId, ChannelId: trace.ChannelId}) //nolint: gosimple
+	}
+
+	// Update message fields.
+	msg.SourcePort, msg.SourceChannel = token.Denom.Trace[0].PortId, token.Denom.Trace[0].ChannelId
+	msg.Forwarding.Hops = append(unwindHops, msg.Forwarding.Hops...)
+	msg.Forwarding.Unwind = false
+
+	if err := msg.ValidateBasic(); err != nil {
+		return nil, err
+	}
+	return msg, nil
+}
diff --git a/modules/apps/transfer/keeper/msg_server_test.go b/modules/apps/transfer/keeper/msg_server_test.go
@@ -13,6 +13,7 @@ import (
 	abci "github.com/cometbft/cometbft/abci/types"
 
 	"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"
 	ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
@@ -238,3 +239,122 @@ func (suite *KeeperTestSuite) TestUpdateParams() {
 		})
 	}
 }
+
+func (suite *KeeperTestSuite) TestUnwindHops() {
+	var msg *types.MsgTransfer
+	var path *ibctesting.Path
+	denom := types.NewDenom(ibctesting.TestCoin.Denom, types.NewTrace(ibctesting.MockPort, "channel-0"), types.NewTrace(ibctesting.MockPort, "channel-1"))
+	coins := sdk.NewCoins(sdk.NewCoin(denom.IBCDenom(), ibctesting.TestCoin.Amount))
+	testCases := []struct {
+		name         string
+		malleate     func()
+		assertResult func(modified *types.MsgTransfer, err error)
+	}{
+		{
+			"success",
+			func() {
+				suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), denom)
+			},
+			func(modified *types.MsgTransfer, err error) {
+				suite.Require().NoError(err, "got unexpected error from unwindHops")
+				msg.SourceChannel = denom.Trace[0].PortId
+				msg.SourcePort = denom.Trace[0].ChannelId
+				msg.Forwarding = types.NewForwarding(false, types.Hop{PortId: denom.Trace[1].PortId, ChannelId: denom.Trace[1].ChannelId})
+				suite.Require().Equal(*msg, *modified, "expected msg and modified msg are different")
+			},
+		},
+		{
+			"success: multiple unwind hops",
+			func() {
+				denom.Trace = append(denom.Trace, types.NewTrace(ibctesting.MockPort, "channel-2"), types.NewTrace(ibctesting.MockPort, "channel-3"))
+				coins = sdk.NewCoins(sdk.NewCoin(denom.IBCDenom(), ibctesting.TestCoin.Amount))
+				suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), denom)
+				msg.Tokens = coins
+			},
+			func(modified *types.MsgTransfer, err error) {
+				suite.Require().NoError(err, "got unexpected error from unwindHops")
+				msg.SourceChannel = denom.Trace[0].PortId
+				msg.SourcePort = denom.Trace[0].ChannelId
+				msg.Forwarding = types.NewForwarding(false,
+					types.Hop{PortId: denom.Trace[3].PortId, ChannelId: denom.Trace[3].ChannelId},
+					types.Hop{PortId: denom.Trace[2].PortId, ChannelId: denom.Trace[2].ChannelId},
+					types.Hop{PortId: denom.Trace[1].PortId, ChannelId: denom.Trace[1].ChannelId},
+				)
+				suite.Require().Equal(*msg, *modified, "expected msg and modified msg are different")
+			},
+		},
+		{
+			"success - unwind hops are added to existing hops",
+			func() {
+				suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), denom)
+				msg.Forwarding = types.NewForwarding(true, types.Hop{PortId: ibctesting.MockPort, ChannelId: "channel-2"})
+			},
+			func(modified *types.MsgTransfer, err error) {
+				suite.Require().NoError(err, "got unexpected error from unwindHops")
+				msg.SourceChannel = denom.Trace[0].PortId
+				msg.SourcePort = denom.Trace[0].ChannelId
+				msg.Forwarding = types.NewForwarding(false,
+					types.Hop{PortId: denom.Trace[1].PortId, ChannelId: denom.Trace[1].ChannelId},
+					types.Hop{PortId: ibctesting.MockPort, ChannelId: "channel-2"},
+				)
+				suite.Require().Equal(*msg, *modified, "expected msg and modified msg are different")
+			},
+		},
+		{
+			"failure: no denom set on keeper",
+			func() {},
+			func(modified *types.MsgTransfer, err error) {
+				suite.Require().ErrorIs(err, types.ErrDenomNotFound)
+			},
+		},
+		{
+			"failure: validateBasic() fails due to invalid channelID",
+			func() {
+				denom.Trace[0].ChannelId = "channel/0"
+				coins = sdk.NewCoins(sdk.NewCoin(denom.IBCDenom(), ibctesting.TestCoin.Amount))
+				msg.Tokens = coins
+				suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), denom)
+			},
+			func(modified *types.MsgTransfer, err error) {
+				suite.Require().ErrorContains(err, "invalid source channel ID")
+			},
+		},
+		{
+			"failure: denom is native",
+			func() {
+				denom.Trace = nil
+				coins = sdk.NewCoins(sdk.NewCoin(denom.IBCDenom(), ibctesting.TestCoin.Amount))
+				msg.Tokens = coins
+				suite.chainA.GetSimApp().TransferKeeper.SetDenom(suite.chainA.GetContext(), denom)
+			},
+			func(modified *types.MsgTransfer, err error) {
+				suite.Require().ErrorIs(err, types.ErrInvalidForwarding)
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		suite.Run(tc.name, func() {
+			suite.SetupTest()
+
+			path = ibctesting.NewTransferPath(suite.chainA, suite.chainB)
+			path.Setup()
+
+			msg = types.NewMsgTransfer(
+				path.EndpointA.ChannelConfig.PortID,
+				path.EndpointA.ChannelID,
+				coins,
+				suite.chainA.SenderAccount.GetAddress().String(),
+				suite.chainB.SenderAccount.GetAddress().String(),
+				clienttypes.ZeroHeight(),
+				suite.chainA.GetTimeoutTimestamp(),
+				"memo",
+				types.NewForwarding(true),
+			)
+
+			tc.malleate()
+			gotMsg, err := suite.chainA.GetSimApp().TransferKeeper.UnwindHops(suite.chainA.GetContext(), msg)
+			tc.assertResult(gotMsg, err)
+		})
+	}
+}