refactor: set gas limit associated to the ZRC20 for cctx revert outbo…

…und (#1317)

* initialize

* udpate expected keeper

* implement gas limit method

* change revert tx gas limit

* make generate

* initialize test

* get gas limit test

---------

Co-authored-by: Charlie Chen <[email protected]>

x/crosschain/keeper/cctx_utils.go

@@ -1,16 +1,17 @@
 package keeper
 
 import (
-	"errors"
 	"fmt"
 
 	cosmoserrors "cosmossdk.io/errors"
 	"cosmossdk.io/math"
+	"github.com/pkg/errors"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	ethcommon "github.com/ethereum/go-ethereum/common"
 	"github.com/zeta-chain/zetacore/common"
 	"github.com/zeta-chain/zetacore/x/crosschain/types"
+	fungibletypes "github.com/zeta-chain/zetacore/x/fungible/types"
 	zetaObserverTypes "github.com/zeta-chain/zetacore/x/observer/types"
 )
 
@@ -86,3 +87,37 @@ func (k Keeper) RefundAmountOnZetaChain(ctx sdk.Context, cctx types.CrossChainTx
 
 	return nil
 }
+
+// GetRevertGasLimit returns the gas limit for the revert transaction in a CCTX
+// It returns 0 if there is no error but the gas limit can't be determined from the CCTX data
+func (k Keeper) GetRevertGasLimit(ctx sdk.Context, cctx types.CrossChainTx) (uint64, error) {
+	if cctx.InboundTxParams == nil {
+		return 0, nil
+	}
+
+	if cctx.InboundTxParams.CoinType == common.CoinType_Gas {
+		// get the gas limit of the gas token
+		fc, found := k.fungibleKeeper.GetGasCoinForForeignCoin(ctx, cctx.InboundTxParams.SenderChainId)
+		if !found {
+			return 0, types.ErrForeignCoinNotFound
+		}
+		gasLimit, err := k.fungibleKeeper.QueryGasLimit(ctx, ethcommon.HexToAddress(fc.Zrc20ContractAddress))
+		if err != nil {
+			return 0, errors.Wrap(fungibletypes.ErrContractCall, err.Error())
+		}
+		return gasLimit.Uint64(), nil
+	} else if cctx.InboundTxParams.CoinType == common.CoinType_ERC20 {
+		// get the gas limit of the associated asset
+		fc, found := k.fungibleKeeper.GetForeignCoinFromAsset(ctx, cctx.InboundTxParams.Asset, cctx.InboundTxParams.SenderChainId)
+		if !found {
+			return 0, types.ErrForeignCoinNotFound
+		}
+		gasLimit, err := k.fungibleKeeper.QueryGasLimit(ctx, ethcommon.HexToAddress(fc.Zrc20ContractAddress))
+		if err != nil {
+			return 0, errors.Wrap(fungibletypes.ErrContractCall, err.Error())
+		}
+		return gasLimit.Uint64(), nil
+	}
+
+	return 0, nil
+}

x/crosschain/keeper/cctx_utils_test.go

@@ -1,6 +1,7 @@
 package keeper_test
 
 import (
+	"math/big"
 	"testing"
 
 	"cosmossdk.io/math"
@@ -138,3 +139,142 @@ func TestKeeper_RefundAmountOnZetaChain(t *testing.T) {
 		require.ErrorContains(t, err, "zrc not found")
 	})
 }
+
+func TestGetRevertGasLimit(t *testing.T) {
+	t.Run("should return 0 if no inbound tx params", func(t *testing.T) {
+		k, ctx, _, _ := keepertest.CrosschainKeeper(t)
+
+		gasLimit, err := k.GetRevertGasLimit(ctx, types.CrossChainTx{})
+		require.NoError(t, err)
+		require.Equal(t, uint64(0), gasLimit)
+	})
+
+	t.Run("should return 0 if coin type is not gas or erc20", func(t *testing.T) {
+		k, ctx, _, _ := keepertest.CrosschainKeeper(t)
+
+		gasLimit, err := k.GetRevertGasLimit(ctx, types.CrossChainTx{
+			InboundTxParams: &types.InboundTxParams{
+				CoinType: common.CoinType_Zeta,
+			}})
+		require.NoError(t, err)
+		require.Equal(t, uint64(0), gasLimit)
+	})
+
+	t.Run("should return the gas limit of the gas token", func(t *testing.T) {
+		k, ctx, sdkk, zk := keepertest.CrosschainKeeper(t)
+		k.GetAuthKeeper().GetModuleAccount(ctx, fungibletypes.ModuleName)
+
+		chainID := getValidEthChainID(t)
+		deploySystemContracts(t, ctx, zk.FungibleKeeper, sdkk.EvmKeeper)
+		gas := setupGasCoin(t, ctx, zk.FungibleKeeper, sdkk.EvmKeeper, chainID, "foo", "FOO")
+
+		_, err := zk.FungibleKeeper.UpdateZRC20GasLimit(ctx, gas, big.NewInt(42))
+		require.NoError(t, err)
+
+		gasLimit, err := k.GetRevertGasLimit(ctx, types.CrossChainTx{
+			InboundTxParams: &types.InboundTxParams{
+				CoinType:      common.CoinType_Gas,
+				SenderChainId: chainID,
+			}})
+		require.NoError(t, err)
+		require.Equal(t, uint64(42), gasLimit)
+	})
+
+	t.Run("should return the gas limit of the associated asset", func(t *testing.T) {
+		k, ctx, sdkk, zk := keepertest.CrosschainKeeper(t)
+		k.GetAuthKeeper().GetModuleAccount(ctx, fungibletypes.ModuleName)
+
+		chainID := getValidEthChainID(t)
+		deploySystemContracts(t, ctx, zk.FungibleKeeper, sdkk.EvmKeeper)
+		asset := sample.EthAddress().String()
+		zrc20Addr := deployZRC20(
+			t,
+			ctx,
+			zk.FungibleKeeper,
+			sdkk.EvmKeeper,
+			chainID,
+			"bar",
+			asset,
+			"bar",
+		)
+
+		_, err := zk.FungibleKeeper.UpdateZRC20GasLimit(ctx, zrc20Addr, big.NewInt(42))
+		require.NoError(t, err)
+
+		gasLimit, err := k.GetRevertGasLimit(ctx, types.CrossChainTx{
+			InboundTxParams: &types.InboundTxParams{
+				CoinType:      common.CoinType_ERC20,
+				SenderChainId: chainID,
+				Asset:         asset,
+			}})
+		require.NoError(t, err)
+		require.Equal(t, uint64(42), gasLimit)
+	})
+
+	t.Run("should fail if no gas coin found", func(t *testing.T) {
+		k, ctx, _, _ := keepertest.CrosschainKeeper(t)
+
+		_, err := k.GetRevertGasLimit(ctx, types.CrossChainTx{
+			InboundTxParams: &types.InboundTxParams{
+				CoinType:      common.CoinType_Gas,
+				SenderChainId: 999999,
+			}})
+		require.ErrorIs(t, err, types.ErrForeignCoinNotFound)
+	})
+
+	t.Run("should fail if query gas limit for gas coin fails", func(t *testing.T) {
+		k, ctx, _, zk := keepertest.CrosschainKeeper(t)
+		k.GetAuthKeeper().GetModuleAccount(ctx, fungibletypes.ModuleName)
+
+		chainID := getValidEthChainID(t)
+
+		zk.FungibleKeeper.SetForeignCoins(ctx, fungibletypes.ForeignCoins{
+			Zrc20ContractAddress: sample.EthAddress().String(),
+			ForeignChainId:       chainID,
+			CoinType:             common.CoinType_Gas,
+		})
+
+		// no contract deployed therefore will fail
+		_, err := k.GetRevertGasLimit(ctx, types.CrossChainTx{
+			InboundTxParams: &types.InboundTxParams{
+				CoinType:      common.CoinType_Gas,
+				SenderChainId: chainID,
+			}})
+		require.ErrorIs(t, err, fungibletypes.ErrContractCall)
+	})
+
+	t.Run("should fail if no asset found", func(t *testing.T) {
+		k, ctx, _, _ := keepertest.CrosschainKeeper(t)
+
+		_, err := k.GetRevertGasLimit(ctx, types.CrossChainTx{
+			InboundTxParams: &types.InboundTxParams{
+				CoinType:      common.CoinType_ERC20,
+				SenderChainId: 999999,
+			}})
+		require.ErrorIs(t, err, types.ErrForeignCoinNotFound)
+	})
+
+	t.Run("should fail if query gas limit for asset fails", func(t *testing.T) {
+		k, ctx, _, zk := keepertest.CrosschainKeeper(t)
+		k.GetAuthKeeper().GetModuleAccount(ctx, fungibletypes.ModuleName)
+
+		chainID := getValidEthChainID(t)
+		asset := sample.EthAddress().String()
+
+		zk.FungibleKeeper.SetForeignCoins(ctx, fungibletypes.ForeignCoins{
+			Zrc20ContractAddress: sample.EthAddress().String(),
+			ForeignChainId:       chainID,
+			CoinType:             common.CoinType_ERC20,
+			Asset:                asset,
+		})
+
+		// no contract deployed therefore will fail
+		_, err := k.GetRevertGasLimit(ctx, types.CrossChainTx{
+			InboundTxParams: &types.InboundTxParams{
+				CoinType:      common.CoinType_ERC20,
+				SenderChainId: chainID,
+				Asset:         asset,
+			}})
+		require.ErrorIs(t, err, fungibletypes.ErrContractCall)
+	})
+}

x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx.go

@@ -143,16 +143,26 @@ func (k msgServer) VoteOnObservedInboundTx(goCtx context.Context, msg *types.Msg
 				cctx.CctxStatus.ChangeStatus(types.CctxStatus_Aborted, "invalid sender chain")
 				return &types.MsgVoteOnObservedInboundTxResponse{}, nil
 			}
+
+			gasLimit, err := k.GetRevertGasLimit(ctx, cctx)
+			if err != nil {
+				cctx.CctxStatus.ChangeStatus(types.CctxStatus_Aborted, "can't get revert tx gas limit"+err.Error())
+				return &types.MsgVoteOnObservedInboundTxResponse{}, nil
+			}
+			if gasLimit == 0 {
+				// use same gas limit of outbound as a fallback -- should not happen
+				gasLimit = msg.GasLimit
+			}
+
 			// create new OutboundTxParams for the revert
-			cctx.OutboundTxParams = append(cctx.OutboundTxParams, &types.OutboundTxParams{
-				Receiver:        cctx.InboundTxParams.Sender,
-				ReceiverChainId: cctx.InboundTxParams.SenderChainId,
-				Amount:          cctx.InboundTxParams.Amount,
-				CoinType:        cctx.InboundTxParams.CoinType,
-				// use same gas limit as outbound
-				//TODO: determine a specific revert gas limit https://github.com/zeta-chain/node/issues/1065
-				OutboundTxGasLimit: msg.GasLimit,
-			})
+			revertTxParams := &types.OutboundTxParams{
+				Receiver:           cctx.InboundTxParams.Sender,
+				ReceiverChainId:    cctx.InboundTxParams.SenderChainId,
+				Amount:             cctx.InboundTxParams.Amount,
+				CoinType:           cctx.InboundTxParams.CoinType,
+				OutboundTxGasLimit: gasLimit,
+			}
+			cctx.OutboundTxParams = append(cctx.OutboundTxParams, revertTxParams)
 
 			// we create a new cached context, and we don't commit the previous one with EVM deposit
 			tmpCtx, commit := ctx.CacheContext()

x/crosschain/keeper/keeper_cross_chain_tx_vote_outbound_tx.go

@@ -2,6 +2,7 @@ package keeper
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"math/big"
 
@@ -160,17 +161,27 @@ func (k msgServer) VoteOnObservedOutboundTx(goCtx context.Context, msg *types.Ms
 			} else {
 				switch oldStatus {
 				case types.CctxStatus_PendingOutbound:
+
+					gasLimit, err := k.GetRevertGasLimit(ctx, cctx)
+					if err != nil {
+						return errors.New("can't get revert tx gas limit" + err.Error())
+					}
+					if gasLimit == 0 {
+						// use same gas limit of outbound as a fallback -- should not happen
+						gasLimit = cctx.OutboundTxParams[0].OutboundTxGasLimit
+					}
+
 					// create new OutboundTxParams for the revert
-					cctx.OutboundTxParams = append(cctx.OutboundTxParams, &types.OutboundTxParams{
-						Receiver:        cctx.InboundTxParams.Sender,
-						ReceiverChainId: cctx.InboundTxParams.SenderChainId,
-						Amount:          cctx.InboundTxParams.Amount,
-						CoinType:        cctx.InboundTxParams.CoinType,
-						// NOTE(pwu): revert gas limit = initial outbound gas limit set by user
-						//TODO: determine a specific revert gas limit https://github.com/zeta-chain/node/issues/1065
-						OutboundTxGasLimit: cctx.OutboundTxParams[0].OutboundTxGasLimit,
-					})
-					err := k.PayGasAndUpdateCctx(
+					revertTxParams := &types.OutboundTxParams{
+						Receiver:           cctx.InboundTxParams.Sender,
+						ReceiverChainId:    cctx.InboundTxParams.SenderChainId,
+						Amount:             cctx.InboundTxParams.Amount,
+						CoinType:           cctx.InboundTxParams.CoinType,
+						OutboundTxGasLimit: gasLimit,
+					}
+					cctx.OutboundTxParams = append(cctx.OutboundTxParams, revertTxParams)
+
+					err = k.PayGasAndUpdateCctx(
 						tmpCtx,
 						cctx.InboundTxParams.SenderChainId,
 						&cctx,

x/crosschain/types/expected_keepers.go

@@ -75,6 +75,7 @@ type FungibleKeeper interface {
 	SetForeignCoins(ctx sdk.Context, foreignCoins fungibletypes.ForeignCoins)
 	GetAllForeignCoinsForChain(ctx sdk.Context, foreignChainID int64) (list []fungibletypes.ForeignCoins)
 	GetForeignCoinFromAsset(ctx sdk.Context, asset string, chainID int64) (fungibletypes.ForeignCoins, bool)
+	GetGasCoinForForeignCoin(ctx sdk.Context, chainID int64) (fungibletypes.ForeignCoins, bool)
 	GetSystemContract(ctx sdk.Context) (val fungibletypes.SystemContract, found bool)
 	QuerySystemContractGasCoinZRC20(ctx sdk.Context, chainID *big.Int) (eth.Address, error)
 	GetUniswapV2Router02Address(ctx sdk.Context) (eth.Address, error)