Problem: channel id is not supported for send-to-ibc event (#1005)

* support channel id for send-to-ibc

* fix conflict

* fix lint

* fix integration test

* remove unecessary check

---------

Co-authored-by: yihuang <huang@crypto.com>

CHANGELOG.md

@@ -16,6 +16,7 @@
 - [cronos#842](https://github.com/crypto-org-chain/cronos/pull/842) Add upgrade handler for v2.0.0-testnet3.
 - [cronos#795](https://github.com/crypto-org-chain/cronos/pull/795) Support permissions in cronos.
 - [cronos#997](https://github.com/crypto-org-chain/cronos/pull/997) Fix logic to support proxy contract for cronos originated crc20.
+- [cronos#1005](https://github.com/crypto-org-chain/cronos/pull/1005) Support specify channel id for send-to-ibc event in case of source token.
 
 ### Bug Fixes
 

integration_tests/test_ibc.py

@@ -270,10 +270,7 @@ def check_chainmain_balance_change():
         {"from": ADDRS["validator"]}
     )
     txreceipt = send_transaction(w3, tx)
-    assert txreceipt.status == 1, "should success"
-    chainmain_receiver_balance = amount
-    wait_for_fn("check balance change", check_chainmain_balance_change)
-    assert chainmain_receiver_new_balance == amount + 1
+    assert txreceipt.status == 0, "should fail"
 
     # send back the token to cronos
     # check receiver balance

x/cronos/keeper/evmhandlers/send_cro_to_ibc.go

@@ -86,7 +86,7 @@ func (h SendCroToIbcHandler) Handle(
 		return err
 	}
 	// Initiate IBC transfer from sender account
-	if err = h.cronosKeeper.IbcTransferCoins(ctx, sender.String(), recipient, coins); err != nil {
+	if err = h.cronosKeeper.IbcTransferCoins(ctx, sender.String(), recipient, coins, ""); err != nil {
 		return err
 	}
 	return nil

x/cronos/keeper/evmhandlers/send_to_ibc.go

@@ -78,7 +78,7 @@ func (h SendToIbcHandler) Handle(
 	sender := unpacked[0].(common.Address)
 	recipient := unpacked[1].(string)
 	amount := unpacked[2].(*big.Int)
-	return h.handle(ctx, contract, sender, recipient, amount)
+	return h.handle(ctx, contract, sender, recipient, amount, nil)
 }
 
 func (h SendToIbcHandler) handle(
@@ -87,6 +87,7 @@ func (h SendToIbcHandler) handle(
 	senderAddress common.Address,
 	recipient string,
 	amountInt *big.Int,
+	id *big.Int,
 ) error {
 	denom, found := h.cronosKeeper.GetDenomByContract(ctx, contract)
 	if !found {
@@ -118,8 +119,13 @@ func (h SendToIbcHandler) handle(
 			return err
 		}
 	}
+
+	channelId := ""
+	if id != nil {
+		channelId = "channel-" + id.String()
+	}
 	// Initiate IBC transfer from sender account
-	if err = h.cronosKeeper.IbcTransferCoins(ctx, sender.String(), recipient, coins); err != nil {
+	if err = h.cronosKeeper.IbcTransferCoins(ctx, sender.String(), recipient, coins, channelId); err != nil {
 		return err
 	}
 	return nil

x/cronos/keeper/evmhandlers/send_to_ibc_v2.go

@@ -92,10 +92,10 @@ func (h SendToIbcV2Handler) Handle(
 
 	// needs to crope the extra bytes in the topic by using BytesToAddress
 	sender := common.BytesToAddress(topics[1].Bytes())
+	channelId := new(big.Int).SetBytes(topics[2].Bytes())
 	recipient := unpacked[0].(string)
 	amount := unpacked[1].(*big.Int)
-	// channelId := uint256(topics[2].Bytes())
 	// extraData := unpacked[2].([]byte)
 
-	return h.handle(ctx, contract, sender, recipient, amount)
+	return h.handle(ctx, contract, sender, recipient, amount, channelId)
 }

x/cronos/keeper/ibc.go

@@ -13,6 +13,7 @@ import (
 	errorsmod "cosmossdk.io/errors"
 	ibctransfertypes "github.com/cosmos/ibc-go/v5/modules/apps/transfer/types"
 	ibcclienttypes "github.com/cosmos/ibc-go/v5/modules/core/02-client/types"
+	channeltypes "github.com/cosmos/ibc-go/v5/modules/core/04-channel/types"
 	"github.com/crypto-org-chain/cronos/v2/x/cronos/types"
 )
 
@@ -75,7 +76,7 @@ func (k Keeper) ConvertVouchersToEvmCoins(ctx sdk.Context, from string, coins sd
 	return nil
 }
 
-func (k Keeper) IbcTransferCoins(ctx sdk.Context, from, destination string, coins sdk.Coins) error {
+func (k Keeper) IbcTransferCoins(ctx sdk.Context, from, destination string, coins sdk.Coins, channelId string) error {
 	acc, err := sdk.AccAddressFromBech32(from)
 	if err != nil {
 		return err
@@ -121,7 +122,8 @@ func (k Keeper) IbcTransferCoins(ctx sdk.Context, from, destination string, coin
 				return err
 			}
 
-			err = k.ibcSendTransfer(ctx, acc, destination, ibcCoin)
+			// No need to specify the channelId because it's not a source token
+			err = k.ibcSendTransfer(ctx, acc, destination, ibcCoin, "")
 			if err != nil {
 				return err
 			}
@@ -134,7 +136,7 @@ func (k Keeper) IbcTransferCoins(ctx sdk.Context, from, destination string, coin
 			if !found {
 				return fmt.Errorf("coin %s is not supported", c.Denom)
 			}
-			err = k.ibcSendTransfer(ctx, acc, destination, c)
+			err = k.ibcSendTransfer(ctx, acc, destination, c, channelId)
 			if err != nil {
 				return err
 			}
@@ -155,22 +157,19 @@ func (k Keeper) IbcTransferCoins(ctx sdk.Context, from, destination string, coin
 	return nil
 }
 
-func (k Keeper) ibcSendTransfer(ctx sdk.Context, sender sdk.AccAddress, destination string, coin sdk.Coin) error {
-	// We extract the channel id from the coin denom
-	channelDenom := ""
+func (k Keeper) ibcSendTransfer(ctx sdk.Context, sender sdk.AccAddress, destination string, coin sdk.Coin, channelId string) error {
 	if types.IsSourceCoin(coin.Denom) {
-		// If coin is source, we can only transfer to crypto.org chain
-		// we are using the cro denom to extract the channel id
-		// TODO: support arbitrary channel?
-		channelDenom = k.GetParams(ctx).IbcCroDenom
+		if !channeltypes.IsValidChannelID(channelId) {
+			return errors.New("invalid channel id for ibc transfer of source token")
+		}
 	} else {
 		// If it is not source, then coin is a voucher so we can extract the channel id from the denom
-		channelDenom = coin.Denom
-	}
-
-	channelID, err := k.GetSourceChannelID(ctx, channelDenom)
-	if err != nil {
-		return err
+		channelDenom := coin.Denom
+		sourceChannelID, err := k.GetSourceChannelID(ctx, channelDenom)
+		if err != nil {
+			return err
+		}
+		channelId = sourceChannelID
 	}
 
 	// Transfer coins to receiver through IBC
@@ -182,7 +181,7 @@ func (k Keeper) ibcSendTransfer(ctx sdk.Context, sender sdk.AccAddress, destinat
 	return k.transferKeeper.SendTransfer(
 		ctx,
 		ibctransfertypes.PortID,
-		channelID,
+		channelId,
 		coin,
 		sender,
 		destination,

x/cronos/keeper/ibc_test.go

@@ -16,7 +16,10 @@ import (
 	"github.com/evmos/ethermint/crypto/ethsecp256k1"
 )
 
-const CorrectIbcDenom = "ibc/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+const (
+	CorrectIbcDenom    = "ibc/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+	CorrectCronosDenom = "cronos0xc1b37f2abdb778f540fa5db8e1fd2eadfc9a05ed"
+)
 
 func (suite *KeeperTestSuite) TestConvertVouchersToEvmCoins() {
 	privKey, err := ethsecp256k1.GenerateKey()
@@ -145,6 +148,7 @@ func (suite *KeeperTestSuite) TestIbcTransferCoins() {
 		from          string
 		to            string
 		coin          sdk.Coins
+		channelId     string
 		malleate      func()
 		expectedError error
 		postCheck     func()
@@ -154,6 +158,7 @@ func (suite *KeeperTestSuite) TestIbcTransferCoins() {
 			"test",
 			"to",
 			sdk.NewCoins(sdk.NewCoin(suite.evmParam.EvmDenom, sdk.NewInt(1))),
+			"channel-0",
 			func() {},
 			errors.New("decoding bech32 failed: invalid bech32 string length 4"),
 			func() {},
@@ -163,6 +168,7 @@ func (suite *KeeperTestSuite) TestIbcTransferCoins() {
 			"",
 			"to",
 			sdk.NewCoins(sdk.NewCoin(suite.evmParam.EvmDenom, sdk.NewInt(1))),
+			"channel-0",
 			func() {},
 			errors.New("empty address string is not allowed"),
 			func() {},
@@ -172,6 +178,7 @@ func (suite *KeeperTestSuite) TestIbcTransferCoins() {
 			address.String(),
 			"to",
 			sdk.NewCoins(sdk.NewCoin("ibc/BAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", sdk.NewInt(1))),
+			"channel-0",
 			func() {},
 			errors.New("coin ibc/BAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA is not supported"),
 			func() {},
@@ -181,6 +188,7 @@ func (suite *KeeperTestSuite) TestIbcTransferCoins() {
 			address.String(),
 			"to",
 			sdk.NewCoins(sdk.NewCoin("fake", sdk.NewInt(1))),
+			"channel-0",
 			func() {},
 			errors.New("the coin fake is neither an ibc voucher or a cronos token"),
 			func() {},
@@ -190,6 +198,7 @@ func (suite *KeeperTestSuite) TestIbcTransferCoins() {
 			address.String(),
 			"to",
 			sdk.NewCoins(sdk.NewCoin(suite.evmParam.EvmDenom, sdk.NewInt(123))),
+			"channel-0",
 			func() {},
 			nil,
 			func() {},
@@ -199,6 +208,7 @@ func (suite *KeeperTestSuite) TestIbcTransferCoins() {
 			address.String(),
 			"to",
 			sdk.NewCoins(sdk.NewCoin(suite.evmParam.EvmDenom, sdk.NewInt(1230000000000))),
+			"channel-0",
 			func() {},
 			errors.New("0aphoton is smaller than 1230000000000aphoton: insufficient funds"),
 			func() {},
@@ -208,6 +218,7 @@ func (suite *KeeperTestSuite) TestIbcTransferCoins() {
 			address.String(),
 			"to",
 			sdk.NewCoins(sdk.NewCoin(suite.evmParam.EvmDenom, sdk.NewInt(1230000000000))),
+			"channel-0",
 			func() {
 				// Mint Coin to user and module
 				suite.MintCoins(address, sdk.NewCoins(sdk.NewCoin(suite.evmParam.EvmDenom, sdk.NewInt(1230000000000))))
@@ -234,6 +245,7 @@ func (suite *KeeperTestSuite) TestIbcTransferCoins() {
 			address.String(),
 			"to",
 			sdk.NewCoins(sdk.NewCoin("incorrect", sdk.NewInt(123))),
+			"channel-0",
 			func() {
 				// Add support for the IBC token
 				suite.app.CronosKeeper.SetAutoContractForDenom(suite.ctx, "incorrect", common.HexToAddress("0x11"))
@@ -247,6 +259,7 @@ func (suite *KeeperTestSuite) TestIbcTransferCoins() {
 			address.String(),
 			"to",
 			sdk.NewCoins(sdk.NewCoin(CorrectIbcDenom, sdk.NewInt(123))),
+			"channel-0",
 			func() {
 				// Mint IBC token for user
 				suite.MintCoins(address, sdk.NewCoins(sdk.NewCoin(CorrectIbcDenom, sdk.NewInt(123))))
@@ -257,6 +270,22 @@ func (suite *KeeperTestSuite) TestIbcTransferCoins() {
 			func() {
 			},
 		},
+		{
+			"Correct address with incorrect IBC token denom",
+			address.String(),
+			"to",
+			sdk.NewCoins(sdk.NewCoin(CorrectCronosDenom, sdk.NewInt(123))),
+			"aaa",
+			func() {
+				// Mint IBC token for user
+				suite.MintCoins(address, sdk.NewCoins(sdk.NewCoin(CorrectCronosDenom, sdk.NewInt(123))))
+				// Add support for the IBC token
+				suite.app.CronosKeeper.SetAutoContractForDenom(suite.ctx, CorrectCronosDenom, common.HexToAddress("0x11"))
+			},
+			errors.New("invalid channel id for ibc transfer of source token"),
+			func() {
+			},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -277,7 +306,7 @@ func (suite *KeeperTestSuite) TestIbcTransferCoins() {
 			suite.app.CronosKeeper = cronosKeeper
 
 			tc.malleate()
-			err := suite.app.CronosKeeper.IbcTransferCoins(suite.ctx, tc.from, tc.to, tc.coin)
+			err := suite.app.CronosKeeper.IbcTransferCoins(suite.ctx, tc.from, tc.to, tc.coin, tc.channelId)
 			if tc.expectedError != nil {
 				suite.Require().EqualError(err, tc.expectedError.Error())
 			} else {

x/cronos/keeper/msg_server.go

@@ -44,7 +44,9 @@ func (k msgServer) ConvertVouchers(goCtx context.Context, msg *types.MsgConvertV
 
 func (k msgServer) TransferTokens(goCtx context.Context, msg *types.MsgTransferTokens) (*types.MsgTransferTokensResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
-	err := k.IbcTransferCoins(ctx, msg.From, msg.To, msg.Coins)
+	// TODO change the msg to be able to specify the channel id
+	// Only sending non source token is supported at the moment
+	err := k.IbcTransferCoins(ctx, msg.From, msg.To, msg.Coins, "")
 	if err != nil {
 		return nil, err
 	}