fix: exclude reverted cctx from rate limiter

changelog.md

@@ -45,6 +45,7 @@
 * [2222](https://github.com/zeta-chain/node/pull/2222) - removed `maxHeightDiff` to let observer scan from Bitcoin height where it left off
 * [2233](https://github.com/zeta-chain/node/pull/2233) - fix `IsSupported` flag not properly updated in zetaclient's context
 * [2243](https://github.com/zeta-chain/node/pull/2243) - fix incorrect bitcoin outbound height in the CCTX outbound parameter
+* [2256](https://github.com/zeta-chain/node/pull/2256) - fix rate limiter falsely included reverted non-withdraw cctxs
 
 ### CI
 

testutil/sample/crosschain.go

@@ -224,7 +224,8 @@ func CustomCctxsInBlockRange(
 	t *testing.T,
 	lowBlock uint64,
 	highBlock uint64,
-	chainID int64,
+	senderChainID int64,
+	receiverChainID int64,
 	coinType coin.CoinType,
 	asset string,
 	amount uint64,
@@ -233,12 +234,13 @@ func CustomCctxsInBlockRange(
 	// create 1 cctx per block
 	for i := lowBlock; i <= highBlock; i++ {
 		nonce := i - 1
-		cctx := CrossChainTx(t, fmt.Sprintf("%d-%d", chainID, nonce))
+		cctx := CrossChainTx(t, fmt.Sprintf("%d-%d", receiverChainID, nonce))
 		cctx.CctxStatus.Status = status
+		cctx.InboundParams.SenderChainId = senderChainID
 		cctx.InboundParams.CoinType = coinType
 		cctx.InboundParams.Asset = asset
 		cctx.InboundParams.ObservedExternalHeight = i
-		cctx.GetCurrentOutboundParam().ReceiverChainId = chainID
+		cctx.GetCurrentOutboundParam().ReceiverChainId = receiverChainID
 		cctx.GetCurrentOutboundParam().Amount = sdk.NewUint(amount)
 		cctx.GetCurrentOutboundParam().TssNonce = nonce
 		cctxs = append(cctxs, cctx)

x/crosschain/keeper/grpc_query_cctx_rate_limit.go

@@ -9,6 +9,7 @@ import (
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 
+	"github.com/zeta-chain/zetacore/pkg/chains"
 	"github.com/zeta-chain/zetacore/x/crosschain/types"
 	observertypes "github.com/zeta-chain/zetacore/x/observer/types"
 )
@@ -55,11 +56,17 @@ func (k Keeper) RateLimiterInput(
 	}
 
 	// if a cctx falls within the rate limiter window
-	isCctxInWindow := func(cctx *types.CrossChainTx) bool {
+	isCCTXInWindow := func(cctx *types.CrossChainTx) bool {
 		// #nosec G701 checked positive
 		return cctx.InboundParams.ObservedExternalHeight >= uint64(leftWindowBoundary)
 	}
 
+	// if a cctx is an outgoing cctx that orginates from ZetaChain
+	// reverted incoming cctx has an external `SenderChainId` and should not be counted
+	isCCTXOutgoing := func(cctx *types.CrossChainTx) bool {
+		return chains.IsZetaChain(cctx.InboundParams.SenderChainId)
+	}
+
 	// it is a past cctx if its nonce < `nonceLow`,
 	isPastCctx := func(cctx *types.CrossChainTx, nonceLow int64) bool {
 		// #nosec G701 always positive
@@ -123,7 +130,8 @@ func (k Keeper) RateLimiterInput(
 			if err != nil {
 				return nil, err
 			}
-			inWindow := isCctxInWindow(cctx)
+			inWindow := isCCTXInWindow(cctx)
+			isOutgoing := isCCTXOutgoing(cctx)
 			isPast := isPastCctx(cctx, pendingNonces.NonceLow)
 
 			// we should at least go backwards by 1000 nonces to pick up missed pending cctxs
@@ -132,8 +140,8 @@ func (k Keeper) RateLimiterInput(
 				break
 			}
 
-			// sum up past cctxs' value within window
-			if inWindow && isPast {
+			// sum up the cctxs' value if the cctx is outgoing, within the window and in the past
+			if inWindow && isOutgoing && isPast {
 				pastCctxsValue = pastCctxsValue.Add(
 					types.ConvertCctxValueToAzeta(chain.ChainId, cctx, gasAssetRateMap, erc20AssetRateMap),
 				)
@@ -197,7 +205,7 @@ func (k Keeper) ListPendingCctxWithinRateLimit(
 	totalPending := uint64(0)
 	totalWithdrawInAzeta := sdkmath.NewInt(0)
 	cctxs := make([]*types.CrossChainTx, 0)
-	chains := k.zetaObserverKeeper.GetSupportedForeignChains(ctx)
+	foreignChains := k.zetaObserverKeeper.GetSupportedForeignChains(ctx)
 
 	// check rate limit flags to decide if we should apply rate limit
 	applyLimit := true
@@ -211,7 +219,7 @@ func (k Keeper) ListPendingCctxWithinRateLimit(
 
 	// fallback to non-rate-limited query if rate limiter is disabled
 	if !applyLimit {
-		for _, chain := range chains {
+		for _, chain := range foreignChains {
 			resp, err := k.ListPendingCctx(
 				ctx,
 				&types.QueryListPendingCctxRequest{ChainId: chain.ChainId, Limit: limit},
@@ -256,15 +264,21 @@ func (k Keeper) ListPendingCctxWithinRateLimit(
 	}
 
 	// if a cctx falls within the rate limiter window
-	isCctxInWindow := func(cctx *types.CrossChainTx) bool {
+	isCCTXInWindow := func(cctx *types.CrossChainTx) bool {
 		// #nosec G701 checked positive
 		return cctx.InboundParams.ObservedExternalHeight >= uint64(leftWindowBoundary)
 	}
 
+	// if a cctx is outgoing from ZetaChain
+	// reverted incoming cctx has an external `SenderChainId` and should not be counted
+	isCCTXOutgoing := func(cctx *types.CrossChainTx) bool {
+		return chains.IsZetaChain(cctx.InboundParams.SenderChainId)
+	}
+
 	// query pending nonces for each foreign chain and get the lowest height of the pending cctxs
 	lowestPendingCctxHeight := int64(0)
 	pendingNoncesMap := make(map[int64]observertypes.PendingNonces)
-	for _, chain := range chains {
+	for _, chain := range foreignChains {
 		pendingNonces, found := k.GetObserverKeeper().GetPendingNonces(ctx, tss.TssPubkey, chain.ChainId)
 		if !found {
 			return nil, status.Error(codes.Internal, "pending nonces not found")
@@ -300,7 +314,7 @@ func (k Keeper) ListPendingCctxWithinRateLimit(
 	}
 
 	// query backwards for potential missed pending cctxs for each foreign chain
-	for _, chain := range chains {
+	for _, chain := range foreignChains {
 		// we should at least query 1000 prior to find any pending cctx that we might have missed
 		// this logic is needed because a confirmation of higher nonce will automatically update the p.NonceLow
 		// therefore might mask some lower nonce cctx that is still pending.
@@ -317,16 +331,17 @@ func (k Keeper) ListPendingCctxWithinRateLimit(
 			if err != nil {
 				return nil, err
 			}
-			inWindow := isCctxInWindow(cctx)
+			inWindow := isCCTXInWindow(cctx)
+			isOutgoing := isCCTXOutgoing(cctx)
 
 			// we should at least go backwards by 1000 nonces to pick up missed pending cctxs
 			// we might go even further back if rate limiter is enabled and the endNonce hasn't hit the left window boundary yet
 			// stop at the left window boundary if the `endNonce` hasn't hit it yet
 			if nonce < endNonce && !inWindow {
 				break
 			}
-			// skip the cctx if rate limit is exceeded but still accumulate the total withdraw value
-			if inWindow &&
+			// sum up the cctxs' value if the cctx is outgoing and within the window
+			if inWindow && isOutgoing &&
 				types.RateLimitExceeded(
 					chain.ChainId,
 					cctx,
@@ -353,7 +368,7 @@ func (k Keeper) ListPendingCctxWithinRateLimit(
 	missedPending := len(cctxs)
 
 	// query forwards for pending cctxs for each foreign chain
-	for _, chain := range chains {
+	for _, chain := range foreignChains {
 		pendingNonces := pendingNoncesMap[chain.ChainId]
 
 		// #nosec G701 always in range
@@ -365,9 +380,10 @@ func (k Keeper) ListPendingCctxWithinRateLimit(
 			if err != nil {
 				return nil, err
 			}
+			isOutgoing := isCCTXOutgoing(cctx)
 
 			// skip the cctx if rate limit is exceeded but still accumulate the total withdraw value
-			if types.RateLimitExceeded(
+			if isOutgoing && types.RateLimitExceeded(
 				chain.ChainId,
 				cctx,
 				gasAssetRateMap,

x/crosschain/keeper/grpc_query_cctx_rate_limit_test.go

@@ -7,6 +7,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/stretchr/testify/require"
 
+	"github.com/zeta-chain/zetacore/pkg/chains"
 	"github.com/zeta-chain/zetacore/pkg/coin"
 	keepertest "github.com/zeta-chain/zetacore/testutil/keeper"
 	"github.com/zeta-chain/zetacore/testutil/sample"
@@ -95,6 +96,7 @@ func setupForeignCoins(
 func TestKeeper_RateLimiterInput(t *testing.T) {
 	// create sample TSS
 	tss := sample.Tss()
+	zetaChainID := chains.ZetaChainMainnet.ChainId
 
 	// create sample zrc20 addresses for ETH, BTC, USDT
 	zrc20ETH := sample.EthAddress().Hex()
@@ -107,6 +109,7 @@ func TestKeeper_RateLimiterInput(t *testing.T) {
 		t,
 		1,
 		999,
+		zetaChainID,
 		ethChainID,
 		coin.CoinType_Gas,
 		"",
@@ -117,6 +120,7 @@ func TestKeeper_RateLimiterInput(t *testing.T) {
 		t,
 		1000,
 		1199,
+		zetaChainID,
 		ethChainID,
 		coin.CoinType_Gas,
 		"",
@@ -130,6 +134,7 @@ func TestKeeper_RateLimiterInput(t *testing.T) {
 		t,
 		1,
 		999,
+		zetaChainID,
 		btcChainID,
 		coin.CoinType_Gas,
 		"",
@@ -140,6 +145,7 @@ func TestKeeper_RateLimiterInput(t *testing.T) {
 		t,
 		1000,
 		1199,
+		zetaChainID,
 		btcChainID,
 		coin.CoinType_Gas,
 		"",
@@ -470,6 +476,7 @@ func TestKeeper_RateLimiterInput_Errors(t *testing.T) {
 func TestKeeper_ListPendingCctxWithinRateLimit(t *testing.T) {
 	// create sample TSS
 	tss := sample.Tss()
+	zetaChainID := chains.ZetaChainMainnet.ChainId
 
 	// create sample zrc20 addresses for ETH, BTC, USDT
 	zrc20ETH := sample.EthAddress().Hex()
@@ -482,6 +489,7 @@ func TestKeeper_ListPendingCctxWithinRateLimit(t *testing.T) {
 		t,
 		1,
 		999,
+		zetaChainID,
 		ethChainID,
 		coin.CoinType_Gas,
 		"",
@@ -492,19 +500,46 @@ func TestKeeper_ListPendingCctxWithinRateLimit(t *testing.T) {
 		t,
 		1000,
 		1199,
+		zetaChainID,
 		ethChainID,
 		coin.CoinType_Gas,
 		"",
 		uint64(1e15),
 		types.CctxStatus_PendingOutbound,
 	)
 
+	// create Eth chain 999 reverted and 200 pending revert cctxs for rate limiter test
+	// these cctxs should be just ignored by the rate limiter as we can't compare their `ObservedExternalHeight` with window boundary
+	ethRevertedCctxs := sample.CustomCctxsInBlockRange(
+		t,
+		1,
+		999,
+		ethChainID,
+		ethChainID,
+		coin.CoinType_Gas,
+		"",
+		uint64(1e15),
+		types.CctxStatus_Reverted,
+	)
+	ethPendingRevertCctxs := sample.CustomCctxsInBlockRange(
+		t,
+		1000,
+		1199,
+		ethChainID,
+		ethChainID,
+		coin.CoinType_Gas,
+		"",
+		uint64(1e15),
+		types.CctxStatus_PendingRevert,
+	)
+
 	// create Btc chain 999 mined and 200 pending cctxs for rate limiter test
 	// the number 999 is to make it less than `MaxLookbackNonce` so the LoopBackwards gets the chance to hit nonce 0
 	btcMinedCctxs := sample.CustomCctxsInBlockRange(
 		t,
 		1,
 		999,
+		zetaChainID,
 		btcChainID,
 		coin.CoinType_Gas,
 		"",
@@ -515,6 +550,7 @@ func TestKeeper_ListPendingCctxWithinRateLimit(t *testing.T) {
 		t,
 		1000,
 		1199,
+		zetaChainID,
 		btcChainID,
 		coin.CoinType_Gas,
 		"",
@@ -644,6 +680,42 @@ func TestKeeper_ListPendingCctxWithinRateLimit(t *testing.T) {
 			expectedWithdrawRate:   sdk.NewInt(3e18).String(), // 3 ZETA, (2.5 + 0.5) per block
 			rateLimitExceeded:      false,
 		},
+		{
+			name: "can ignore reverted or pending revert cctxs and retrieve all pending cctx without exceeding rate limit",
+			rateLimitFlags: createTestRateLimiterFlags(
+				500,
+				math.NewUint(10*1e18),
+				zrc20ETH,
+				zrc20BTC,
+				zrc20USDT,
+				"2500",
+				"50000",
+				"0.8",
+			),
+			ethMinedCctxs:   ethRevertedCctxs,      // replace mined cctxs with reverted cctxs, should be ignored
+			ethPendingCctxs: ethPendingRevertCctxs, // replace pending cctxs with pending revert cctxs, should be ignored
+			ethPendingNonces: observertypes.PendingNonces{
+				ChainId:   ethChainID,
+				NonceLow:  1099,
+				NonceHigh: 1199,
+				Tss:       tss.TssPubkey,
+			},
+			btcMinedCctxs:   btcMinedCctxs,
+			btcPendingCctxs: btcPendingCctxs,
+			btcPendingNonces: observertypes.PendingNonces{
+				ChainId:   btcChainID,
+				NonceLow:  1099,
+				NonceHigh: 1199,
+				Tss:       tss.TssPubkey,
+			},
+			currentHeight:          1199,
+			queryLimit:             keeper.MaxPendingCctxs,
+			expectedCctxs:          append(append([]*types.CrossChainTx{}, ethPendingRevertCctxs...), btcPendingCctxs...),
+			expectedTotalPending:   400,
+			expectedWithdrawWindow: 500,                       // the sliding window
+			expectedWithdrawRate:   sdk.NewInt(5e17).String(), // 0.5 ZETA per block, only btc cctxs should be counted
+			rateLimitExceeded:      false,
+		},
 		{
 			name: "can loop backwards all the way to endNonce 0",
 			rateLimitFlags: createTestRateLimiterFlags(

zetaclient/orchestrator/orchestrator_test.go

@@ -208,6 +208,7 @@ func Test_GetPendingCctxsWithinRatelimit(t *testing.T) {
 	// define test foreign chains
 	ethChain := chains.EthChain
 	btcChain := chains.BtcMainnetChain
+	zetaChainID := chains.ZetaChainMainnet.ChainId
 	foreignChains := []chains.Chain{
 		ethChain,
 		btcChain,
@@ -222,6 +223,7 @@ func Test_GetPendingCctxsWithinRatelimit(t *testing.T) {
 		t,
 		1,
 		10,
+		zetaChainID,
 		ethChain.ChainId,
 		coin.CoinType_Gas,
 		"",
@@ -232,6 +234,7 @@ func Test_GetPendingCctxsWithinRatelimit(t *testing.T) {
 		t,
 		11,
 		100,
+		zetaChainID,
 		ethChain.ChainId,
 		coin.CoinType_Gas,
 		"",
@@ -245,6 +248,7 @@ func Test_GetPendingCctxsWithinRatelimit(t *testing.T) {
 		t,
 		1,
 		10,
+		zetaChainID,
 		btcChain.ChainId,
 		coin.CoinType_Gas,
 		"",
@@ -255,6 +259,7 @@ func Test_GetPendingCctxsWithinRatelimit(t *testing.T) {
 		t,
 		11,
 		100,
+		zetaChainID,
 		btcChain.ChainId,
 		coin.CoinType_Gas,
 		"",