fix(crosschain): only use EVM supported chains for gas stability po…

…ol and prevent errors to stop iterating other chains (#1687)

* use supported chain and skip bitcoin

* skip error handling in abci

* changelogs

* lint

* add nosec

* add test for iterate function

* fix lint

* fix lint

* add event

changelog.md

@@ -7,6 +7,12 @@
 
 ### Fixes
 
+* [1687](https://github.com/zeta-chain/node/pull/1687) - only use EVM supported chains for gas stability pool
+
+## Version: v12.2.4
+
+### Fixes
+
 * [1638](https://github.com/zeta-chain/node/issues/1638) - additional check to make sure external chain height always increases
 * [1672](https://github.com/zeta-chain/node/pull/1672) - paying 50% more than base gas price to buffer EIP1559 gas price increase
 * [1642](https://github.com/zeta-chain/node/pull/1642) - Change WhitelistERC20 authorization from group1 to group2

proto/crosschain/events.proto

@@ -58,3 +58,9 @@ message EventOutboundSuccess {
   string new_status = 4;
   string value_received = 5;
 }
+
+message EventCCTXGasPriceIncreased {
+  string cctx_index = 1;
+  string gas_price_increase = 2;
+  string additional_fees = 3;
+}

typescript/crosschain/events_pb.d.ts

@@ -291,3 +291,37 @@ export declare class EventOutboundSuccess extends Message<EventOutboundSuccess>
   static equals(a: EventOutboundSuccess | PlainMessage<EventOutboundSuccess> | undefined, b: EventOutboundSuccess | PlainMessage<EventOutboundSuccess> | undefined): boolean;
 }
 
+/**
+ * @generated from message zetachain.zetacore.crosschain.EventCCTXGasPriceIncreased
+ */
+export declare class EventCCTXGasPriceIncreased extends Message<EventCCTXGasPriceIncreased> {
+  /**
+   * @generated from field: string cctx_index = 1;
+   */
+  cctxIndex: string;
+
+  /**
+   * @generated from field: string gas_price_increase = 2;
+   */
+  gasPriceIncrease: string;
+
+  /**
+   * @generated from field: string additional_fees = 3;
+   */
+  additionalFees: string;
+
+  constructor(data?: PartialMessage<EventCCTXGasPriceIncreased>);
+
+  static readonly runtime: typeof proto3;
+  static readonly typeName = "zetachain.zetacore.crosschain.EventCCTXGasPriceIncreased";
+  static readonly fields: FieldList;
+
+  static fromBinary(bytes: Uint8Array, options?: Partial<BinaryReadOptions>): EventCCTXGasPriceIncreased;
+
+  static fromJson(jsonValue: JsonValue, options?: Partial<JsonReadOptions>): EventCCTXGasPriceIncreased;
+
+  static fromJsonString(jsonString: string, options?: Partial<JsonReadOptions>): EventCCTXGasPriceIncreased;
+
+  static equals(a: EventCCTXGasPriceIncreased | PlainMessage<EventCCTXGasPriceIncreased> | undefined, b: EventCCTXGasPriceIncreased | PlainMessage<EventCCTXGasPriceIncreased> | undefined): boolean;
+}
+

x/crosschain/keeper/abci.go

@@ -4,10 +4,11 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/zeta-chain/zetacore/common"
+
 	cosmoserrors "cosmossdk.io/errors"
 	"cosmossdk.io/math"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/zeta-chain/zetacore/common"
 	"github.com/zeta-chain/zetacore/x/crosschain/types"
 	observertypes "github.com/zeta-chain/zetacore/x/observer/types"
 )
@@ -17,8 +18,21 @@ const (
 	RemainingFeesToStabilityPoolPercent = 95
 )
 
+// CheckAndUpdateCctxGasPriceFunc is a function type for checking and updating the gas price of a cctx
+type CheckAndUpdateCctxGasPriceFunc func(
+	ctx sdk.Context,
+	k Keeper,
+	cctx types.CrossChainTx,
+	flags observertypes.GasPriceIncreaseFlags,
+) (math.Uint, math.Uint, error)
+
 // IterateAndUpdateCctxGasPrice iterates through all cctx and updates the gas price if pending for too long
-func (k Keeper) IterateAndUpdateCctxGasPrice(ctx sdk.Context) error {
+// The function returns the number of cctxs updated and the gas price increase flags used
+func (k Keeper) IterateAndUpdateCctxGasPrice(
+	ctx sdk.Context,
+	chains []*common.Chain,
+	updateFunc CheckAndUpdateCctxGasPriceFunc,
+) (int, observertypes.GasPriceIncreaseFlags) {
 	// fetch the gas price increase flags or use default
 	gasPriceIncreaseFlags := observertypes.DefaultGasPriceIncreaseFlags
 	crosschainFlags, found := k.zetaObserverKeeper.GetCrosschainFlags(ctx)
@@ -28,38 +42,66 @@ func (k Keeper) IterateAndUpdateCctxGasPrice(ctx sdk.Context) error {
 
 	// skip if haven't reached epoch end
 	if ctx.BlockHeight()%gasPriceIncreaseFlags.EpochLength != 0 {
-		return nil
+		return 0, gasPriceIncreaseFlags
 	}
 
-	// iterate all chains' pending cctx
-	chains := common.DefaultChainsList()
+	cctxCount := 0
+
+IterateChains:
 	for _, chain := range chains {
-		res, err := k.CctxListPending(sdk.UnwrapSDKContext(ctx), &types.QueryListCctxPendingRequest{
-			ChainId: chain.ChainId,
-			Limit:   gasPriceIncreaseFlags.MaxPendingCctxs,
-		})
-		if err != nil {
-			return err
-		}
+		// support only external evm chains
+		if common.IsEVMChain(chain.ChainId) && !common.IsZetaChain(chain.ChainId) {
+			res, err := k.CctxListPending(sdk.UnwrapSDKContext(ctx), &types.QueryListCctxPendingRequest{
+				ChainId: chain.ChainId,
+				Limit:   gasPriceIncreaseFlags.MaxPendingCctxs,
+			})
+			if err != nil {
+				ctx.Logger().Info("GasStabilityPool: fetching pending cctx failed",
+					"chainID", chain.ChainId,
+					"err", err.Error(),
+				)
+				continue IterateChains
+			}
 
-		// iterate through all pending cctx
-		for _, pendingCctx := range res.CrossChainTx {
-			if pendingCctx != nil {
-				_, _, err := k.CheckAndUpdateCctxGasPrice(ctx, *pendingCctx, gasPriceIncreaseFlags)
-				if err != nil {
-					return err
+			// iterate through all pending cctx
+			for _, pendingCctx := range res.CrossChainTx {
+				if pendingCctx != nil {
+					gasPriceIncrease, additionalFees, err := updateFunc(ctx, k, *pendingCctx, gasPriceIncreaseFlags)
+					if err != nil {
+						ctx.Logger().Info("GasStabilityPool: updating gas price for pending cctx failed",
+							"cctxIndex", pendingCctx.Index,
+							"err", err.Error(),
+						)
+						continue IterateChains
+					}
+					if !gasPriceIncrease.IsNil() && !gasPriceIncrease.IsZero() {
+						// Emit typed event for gas price increase
+						if err := ctx.EventManager().EmitTypedEvent(
+							&types.EventCCTXGasPriceIncreased{
+								CctxIndex:        pendingCctx.Index,
+								GasPriceIncrease: gasPriceIncrease.String(),
+								AdditionalFees:   additionalFees.String(),
+							}); err != nil {
+							ctx.Logger().Error(
+								"GasStabilityPool: failed to emit EventCCTXGasPriceIncreased",
+								"err", err.Error(),
+							)
+						}
+						cctxCount++
+					}
 				}
 			}
 		}
 	}
 
-	return nil
+	return cctxCount, gasPriceIncreaseFlags
 }
 
 // CheckAndUpdateCctxGasPrice checks if the retry interval is reached and updates the gas price if so
 // The function returns the gas price increase and the additional fees paid from the gas stability pool
-func (k Keeper) CheckAndUpdateCctxGasPrice(
+func CheckAndUpdateCctxGasPrice(
 	ctx sdk.Context,
+	k Keeper,
 	cctx types.CrossChainTx,
 	flags observertypes.GasPriceIncreaseFlags,
 ) (math.Uint, math.Uint, error) {
@@ -108,7 +150,7 @@ func (k Keeper) CheckAndUpdateCctxGasPrice(
 	if err := k.fungibleKeeper.WithdrawFromGasStabilityPool(ctx, chainID, additionalFees.BigInt()); err != nil {
 		return math.ZeroUint(), math.ZeroUint(), cosmoserrors.Wrap(
 			types.ErrNotEnoughFunds,
-			fmt.Sprintf("cannot withdraw %s from gas stability pool", additionalFees.String()),
+			fmt.Sprintf("cannot withdraw %s from gas stability pool, error: %s", additionalFees.String(), err.Error()),
 		)
 	}
 

x/crosschain/keeper/abci_test.go

@@ -6,13 +6,105 @@ import (
 	"time"
 
 	"cosmossdk.io/math"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/stretchr/testify/require"
+	"github.com/zeta-chain/zetacore/common"
 	testkeeper "github.com/zeta-chain/zetacore/testutil/keeper"
+	"github.com/zeta-chain/zetacore/testutil/sample"
+	"github.com/zeta-chain/zetacore/x/crosschain/keeper"
 	"github.com/zeta-chain/zetacore/x/crosschain/types"
 	observertypes "github.com/zeta-chain/zetacore/x/observer/types"
 )
 
-func TestKeeper_CheckAndUpdateCctxGasPrice(t *testing.T) {
+func TestKeeper_IterateAndUpdateCctxGasPrice(t *testing.T) {
+	k, ctx, _, zk := testkeeper.CrosschainKeeper(t)
+
+	// updateFuncMap tracks the calls done with cctx index
+	updateFuncMap := make(map[string]struct{})
+
+	// failMap gives the cctx index that should fail
+	failMap := make(map[string]struct{})
+
+	// updateFunc mocks the update function and keep track of the calls done with cctx index
+	updateFunc := func(
+		ctx sdk.Context,
+		k keeper.Keeper,
+		cctx types.CrossChainTx,
+		flags observertypes.GasPriceIncreaseFlags,
+	) (math.Uint, math.Uint, error) {
+		if _, ok := failMap[cctx.Index]; ok {
+			return math.NewUint(0), math.NewUint(0), errors.New("failed")
+		}
+
+		updateFuncMap[cctx.Index] = struct{}{}
+		return math.NewUint(10), math.NewUint(10), nil
+	}
+
+	// add some evm and non-evm chains
+	supportedChains := []*common.Chain{
+		{ChainId: common.EthChain().ChainId},
+		{ChainId: common.BtcMainnetChain().ChainId},
+		{ChainId: common.BscMainnetChain().ChainId},
+		{ChainId: common.ZetaChainMainnet().ChainId},
+	}
+
+	// set pending cctx
+	tss := sample.Tss()
+	zk.ObserverKeeper.SetTSS(ctx, tss)
+	createCctxWithNonceRange(t, ctx, *k, 10, 15, common.EthChain().ChainId, tss, zk)
+	createCctxWithNonceRange(t, ctx, *k, 20, 25, common.BtcMainnetChain().ChainId, tss, zk)
+	createCctxWithNonceRange(t, ctx, *k, 30, 35, common.BscMainnetChain().ChainId, tss, zk)
+	createCctxWithNonceRange(t, ctx, *k, 40, 45, common.ZetaChainMainnet().ChainId, tss, zk)
+
+	// set a cctx where the update function should fail to test that the next cctx are not updated but the next chains are
+	failMap["1-12"] = struct{}{}
+
+	// test that the default crosschain flags are used when not set and the epoch length is not reached
+	ctx = ctx.WithBlockHeight(observertypes.DefaultCrosschainFlags().GasPriceIncreaseFlags.EpochLength + 1)
+
+	cctxCount, flags := k.IterateAndUpdateCctxGasPrice(ctx, supportedChains, updateFunc)
+	require.Equal(t, 0, cctxCount)
+	require.Equal(t, *observertypes.DefaultCrosschainFlags().GasPriceIncreaseFlags, flags)
+
+	// test that custom crosschain flags are used when set and the epoch length is reached
+	customFlags := observertypes.GasPriceIncreaseFlags{
+		EpochLength:             100,
+		RetryInterval:           time.Minute * 10,
+		GasPriceIncreasePercent: 100,
+		GasPriceIncreaseMax:     200,
+		MaxPendingCctxs:         10,
+	}
+	crosschainFlags := sample.CrosschainFlags()
+	crosschainFlags.GasPriceIncreaseFlags = &customFlags
+	zk.ObserverKeeper.SetCrosschainFlags(ctx, *crosschainFlags)
+
+	cctxCount, flags = k.IterateAndUpdateCctxGasPrice(ctx, supportedChains, updateFunc)
+	require.Equal(t, 0, cctxCount)
+	require.Equal(t, customFlags, flags)
+
+	// test that cctx are iterated and updated when the epoch length is reached
+
+	ctx = ctx.WithBlockHeight(observertypes.DefaultCrosschainFlags().GasPriceIncreaseFlags.EpochLength * 2)
+	cctxCount, flags = k.IterateAndUpdateCctxGasPrice(ctx, supportedChains, updateFunc)
+
+	// 2 eth + 5 bsc = 7
+	require.Equal(t, 7, cctxCount)
+	require.Equal(t, customFlags, flags)
+
+	// check that the update function was called with the cctx index
+	require.Equal(t, 7, len(updateFuncMap))
+	require.Contains(t, updateFuncMap, "1-10")
+	require.Contains(t, updateFuncMap, "1-11")
+
+	require.Contains(t, updateFuncMap, "56-30")
+	require.Contains(t, updateFuncMap, "56-31")
+	require.Contains(t, updateFuncMap, "56-32")
+	require.Contains(t, updateFuncMap, "56-33")
+	require.Contains(t, updateFuncMap, "56-34")
+}
+
+func TestCheckAndUpdateCctxGasPrice(t *testing.T) {
 	sampleTimestamp := time.Now()
 	retryIntervalReached := sampleTimestamp.Add(observertypes.DefaultGasPriceIncreaseFlags.RetryInterval + time.Second)
 	retryIntervalNotReached := sampleTimestamp.Add(observertypes.DefaultGasPriceIncreaseFlags.RetryInterval - time.Second)
@@ -282,7 +374,7 @@ func TestKeeper_CheckAndUpdateCctxGasPrice(t *testing.T) {
 			}
 
 			// check and update gas price
-			gasPriceIncrease, feesPaid, err := k.CheckAndUpdateCctxGasPrice(ctx, tc.cctx, tc.flags)
+			gasPriceIncrease, feesPaid, err := keeper.CheckAndUpdateCctxGasPrice(ctx, *k, tc.cctx, tc.flags)
 
 			if tc.isError {
 				require.Error(t, err)

x/crosschain/keeper/grpc_query_cctx_test.go

@@ -22,14 +22,14 @@ func createCctxWithNonceRange(
 	t *testing.T,
 	ctx sdk.Context,
 	k keeper.Keeper,
-	low int,
-	high int,
+	lowPending int,
+	highPending int,
 	chainID int64,
 	tss observertypes.TSS,
 	zk keepertest.ZetaKeepers,
 ) (cctxs []*types.CrossChainTx) {
-	for i := 0; i < low; i++ {
-		cctx := sample.CrossChainTx(t, fmt.Sprintf("%d", i))
+	for i := 0; i < lowPending; i++ {
+		cctx := sample.CrossChainTx(t, fmt.Sprintf("%d-%d", chainID, i))
 		cctx.CctxStatus.Status = types.CctxStatus_OutboundMined
 		cctx.InboundTxParams.SenderChainId = chainID
 		k.SetCrossChainTx(ctx, *cctx)
@@ -40,8 +40,8 @@ func createCctxWithNonceRange(
 			Tss:       tss.TssPubkey,
 		})
 	}
-	for i := low; i < high; i++ {
-		cctx := sample.CrossChainTx(t, fmt.Sprintf("%d", i))
+	for i := lowPending; i < highPending; i++ {
+		cctx := sample.CrossChainTx(t, fmt.Sprintf("%d-%d", chainID, i))
 		cctx.CctxStatus.Status = types.CctxStatus_PendingOutbound
 		cctx.InboundTxParams.SenderChainId = chainID
 		k.SetCrossChainTx(ctx, *cctx)
@@ -55,8 +55,8 @@ func createCctxWithNonceRange(
 	}
 	zk.ObserverKeeper.SetPendingNonces(ctx, observertypes.PendingNonces{
 		ChainId:   chainID,
-		NonceLow:  int64(low),
-		NonceHigh: int64(high),
+		NonceLow:  int64(lowPending),
+		NonceHigh: int64(highPending),
 		Tss:       tss.TssPubkey,
 	})
 
@@ -135,12 +135,12 @@ func TestKeeper_CctxListPending(t *testing.T) {
 		cctxs := createCctxWithNonceRange(t, ctx, *k, 1000, 2000, chainID, tss, zk)
 
 		// set some cctxs as pending below nonce
-		cctx1, found := k.GetCrossChainTx(ctx, "940")
+		cctx1, found := k.GetCrossChainTx(ctx, "1337-940")
 		require.True(t, found)
 		cctx1.CctxStatus.Status = types.CctxStatus_PendingOutbound
 		k.SetCrossChainTx(ctx, cctx1)
 
-		cctx2, found := k.GetCrossChainTx(ctx, "955")
+		cctx2, found := k.GetCrossChainTx(ctx, "1337-955")
 		require.True(t, found)
 		cctx2.CctxStatus.Status = types.CctxStatus_PendingOutbound
 		k.SetCrossChainTx(ctx, cctx2)

x/crosschain/module.go

@@ -188,10 +188,12 @@ func (AppModule) ConsensusVersion() uint64 { return 4 }
 
 // BeginBlock executes all ABCI BeginBlock logic respective to the crosschain module.
 func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) {
-	err := am.keeper.IterateAndUpdateCctxGasPrice(ctx)
-	if err != nil {
-		ctx.Logger().Info("Error iterating and updating pending cctx gas price", "err", err.Error())
-	}
+	// get all supported chains
+	supportedChains := am.keeper.GetObserverKeeper().GetSupportedChains(ctx)
+
+	// iterate and update gas price for cctx that are pending for too long
+	// error is logged in the function
+	am.keeper.IterateAndUpdateCctxGasPrice(ctx, supportedChains, keeper.CheckAndUpdateCctxGasPrice)
 }
 
 // EndBlock executes all ABCI EndBlock logic respective to the crosschain module. It