From e627adf65be98b6eb7c9a9c11f187d37556987a5 Mon Sep 17 00:00:00 2001 From: Charlie Chen Date: Fri, 24 May 2024 12:51:39 -0500 Subject: [PATCH] ported the rate limiter fix for reverted cctx --- .../keeper/grpc_query_cctx_rate_limit.go | 46 +++++++++++---- .../keeper/grpc_query_cctx_rate_limit_test.go | 57 ++++++++++++++++--- 2 files changed, 84 insertions(+), 19 deletions(-) diff --git a/x/crosschain/keeper/grpc_query_cctx_rate_limit.go b/x/crosschain/keeper/grpc_query_cctx_rate_limit.go index b1aa54c46a..444cee9d70 100644 --- a/x/crosschain/keeper/grpc_query_cctx_rate_limit.go +++ b/x/crosschain/keeper/grpc_query_cctx_rate_limit.go @@ -7,6 +7,7 @@ import ( sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/zeta-chain/zetacore/pkg/chains" "github.com/zeta-chain/zetacore/pkg/coin" "github.com/zeta-chain/zetacore/x/crosschain/types" fungibletypes "github.com/zeta-chain/zetacore/x/fungible/types" @@ -34,7 +35,7 @@ func (k Keeper) ListPendingCctxWithinRateLimit(c context.Context, req *types.Que 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 @@ -48,7 +49,7 @@ func (k Keeper) ListPendingCctxWithinRateLimit(c context.Context, req *types.Que // 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}) if err == nil { cctxs = append(cctxs, resp.CrossChainTx...) @@ -100,15 +101,21 @@ func (k Keeper) ListPendingCctxWithinRateLimit(c context.Context, req *types.Que } // 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.InboundTxParams.InboundTxObservedExternalHeight >= 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.InboundTxParams.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") @@ -144,7 +151,7 @@ func (k Keeper) ListPendingCctxWithinRateLimit(c context.Context, req *types.Que } // 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. @@ -161,7 +168,8 @@ func (k Keeper) ListPendingCctxWithinRateLimit(c context.Context, req *types.Que 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 @@ -169,8 +177,17 @@ func (k Keeper) ListPendingCctxWithinRateLimit(c context.Context, req *types.Que if nonce < endNonce && !inWindow { break } - // skip the cctx if rate limit is exceeded but still accumulate the total withdraw value - if inWindow && rateLimitExceeded(chain.ChainId, cctx, gasCoinRates, erc20CoinRates, foreignCoinMap, &totalWithdrawInAzeta, withdrawLimitInAzeta) { + // sum up the cctxs' value if the cctx is outgoing and within the window + if inWindow && isOutgoing && + rateLimitExceeded( + chain.ChainId, + cctx, + gasCoinRates, + erc20CoinRates, + foreignCoinMap, + &totalWithdrawInAzeta, + withdrawLimitInAzeta, + ) { limitExceeded = true continue } @@ -189,7 +206,7 @@ func (k Keeper) ListPendingCctxWithinRateLimit(c context.Context, req *types.Que 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 @@ -201,9 +218,18 @@ func (k Keeper) ListPendingCctxWithinRateLimit(c context.Context, req *types.Que 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 rateLimitExceeded(chain.ChainId, cctx, gasCoinRates, erc20CoinRates, foreignCoinMap, &totalWithdrawInAzeta, withdrawLimitInAzeta) { + if isOutgoing && rateLimitExceeded( + chain.ChainId, + cctx, + gasCoinRates, + erc20CoinRates, + foreignCoinMap, + &totalWithdrawInAzeta, + withdrawLimitInAzeta, + ) { limitExceeded = true continue } diff --git a/x/crosschain/keeper/grpc_query_cctx_rate_limit_test.go b/x/crosschain/keeper/grpc_query_cctx_rate_limit_test.go index 6418e57c03..e08f06cdaf 100644 --- a/x/crosschain/keeper/grpc_query_cctx_rate_limit_test.go +++ b/x/crosschain/keeper/grpc_query_cctx_rate_limit_test.go @@ -8,6 +8,7 @@ import ( "cosmossdk.io/math" 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" @@ -22,6 +23,9 @@ var ( // local btc chain ID btcChainID = getValidBtcChainID() + + // local zeta chain ID + zetaChainID = chains.ZetaPrivnetChain().ChainId ) // createTestRateLimiterFlags creates a custom rate limiter flags @@ -67,7 +71,8 @@ func createCctxsWithCoinTypeAndHeightRange( t *testing.T, lowBlock uint64, highBlock uint64, - chainID int64, + senderChainID int64, + receiverChainID int64, coinType coin.CoinType, asset string, amount uint64, @@ -76,13 +81,13 @@ func createCctxsWithCoinTypeAndHeightRange( // create 1 pending cctxs per block for i := lowBlock; i <= highBlock; i++ { nonce := i - 1 - cctx := sample.CrossChainTx(t, fmt.Sprintf("%d-%d", chainID, nonce)) + cctx := sample.CrossChainTx(t, fmt.Sprintf("%d-%d", receiverChainID, nonce)) cctx.CctxStatus.Status = status - cctx.InboundTxParams.SenderChainId = chainID + cctx.InboundTxParams.SenderChainId = senderChainID cctx.InboundTxParams.CoinType = coinType cctx.InboundTxParams.Asset = asset cctx.InboundTxParams.InboundTxObservedExternalHeight = i - cctx.GetCurrentOutTxParam().ReceiverChainId = chainID + cctx.GetCurrentOutTxParam().ReceiverChainId = receiverChainID cctx.GetCurrentOutTxParam().Amount = sdk.NewUint(amount) cctx.GetCurrentOutTxParam().OutboundTxTssNonce = nonce cctxs = append(cctxs, cctx) @@ -101,7 +106,7 @@ func setCctxsInKeeper( for _, cctx := range cctxs { k.SetCrossChainTx(ctx, *cctx) zk.ObserverKeeper.SetNonceToCctx(ctx, observertypes.NonceToCctx{ - ChainId: cctx.InboundTxParams.SenderChainId, + ChainId: cctx.GetCurrentOutTxParam().ReceiverChainId, // #nosec G701 always in range for tests Nonce: int64(cctx.GetCurrentOutTxParam().OutboundTxTssNonce), CctxIndex: cctx.Index, @@ -275,13 +280,18 @@ func TestKeeper_ListPendingCctxWithinRateLimit(t *testing.T) { // create Eth 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 - ethMinedCctxs := createCctxsWithCoinTypeAndHeightRange(t, 1, 999, ethChainID, coin.CoinType_Gas, "", uint64(1e15), types.CctxStatus_OutboundMined) - ethPendingCctxs := createCctxsWithCoinTypeAndHeightRange(t, 1000, 1199, ethChainID, coin.CoinType_Gas, "", uint64(1e15), types.CctxStatus_PendingOutbound) + ethMinedCctxs := createCctxsWithCoinTypeAndHeightRange(t, 1, 999, zetaChainID, ethChainID, coin.CoinType_Gas, "", uint64(1e15), types.CctxStatus_OutboundMined) + ethPendingCctxs := createCctxsWithCoinTypeAndHeightRange(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 := createCctxsWithCoinTypeAndHeightRange(t, 1, 999, ethChainID, ethChainID, coin.CoinType_Gas, "", uint64(1e15), types.CctxStatus_Reverted) + ethPendingRevertCctxs := createCctxsWithCoinTypeAndHeightRange(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 := createCctxsWithCoinTypeAndHeightRange(t, 1, 999, btcChainID, coin.CoinType_Gas, "", 1000, types.CctxStatus_OutboundMined) - btcPendingCctxs := createCctxsWithCoinTypeAndHeightRange(t, 1000, 1199, btcChainID, coin.CoinType_Gas, "", 1000, types.CctxStatus_PendingOutbound) + btcMinedCctxs := createCctxsWithCoinTypeAndHeightRange(t, 1, 999, zetaChainID, btcChainID, coin.CoinType_Gas, "", 1000, types.CctxStatus_OutboundMined) + btcPendingCctxs := createCctxsWithCoinTypeAndHeightRange(t, 1000, 1199, zetaChainID, btcChainID, coin.CoinType_Gas, "", 1000, types.CctxStatus_PendingOutbound) // define test cases tests := []struct { @@ -387,6 +397,35 @@ 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(500, math.NewUint(10*1e18), zrc20ETH, zrc20BTC, zrc20USDT, "2500", "50000", "0.8"),