diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index ef58c59c31..3adfa8e37b 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -202,7 +202,7 @@ jobs: build-test: needs: - check_branch - runs-on: buildjet-4vcpu-ubuntu-2004 + runs-on: buildjet-4vcpu-ubuntu-2204 timeout-minutes: 15 concurrency: group: "build-test" @@ -411,7 +411,7 @@ jobs: - e2e-admin-tests - e2e-upgrade-test - check_branch - runs-on: buildjet-4vcpu-ubuntu-2004 + runs-on: ubuntu-20.04 timeout-minutes: 60 environment: release steps: diff --git a/.github/workflows/rc-release.yml b/.github/workflows/rc-release.yml index 04257ef171..26804e8937 100644 --- a/.github/workflows/rc-release.yml +++ b/.github/workflows/rc-release.yml @@ -34,7 +34,7 @@ jobs: echo "The major version found in 'releaseVersion' in app/setup_handlers.go matches this tagged release - Moving Forward!" publish-release: - runs-on: buildjet-4vcpu-ubuntu-2004 + runs-on: buildjet-4vcpu-ubuntu-2204 timeout-minutes: 60 needs: - pre-release-checks diff --git a/changelog.md b/changelog.md index fd4df8a998..5c9920c2e2 100644 --- a/changelog.md +++ b/changelog.md @@ -46,9 +46,10 @@ * [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 - +* [2268](https://github.com/zeta-chain/node/pull/2268) - CI: updated the publish-release pipeline to utilize the Github Actions Ubuntu 20.04 Runners. * [2070](https://github.com/zeta-chain/node/pull/2070) - Added commands to build binaries from the working branch as a live full node rpc to test non-governance changes. * [2119](https://github.com/zeta-chain/node/pull/2119) - Updated the release pipeline to only run on hotfix/ and release/ branches. Added option to only run pre-checks and not cut release as well. Switched approval steps to use environments. * [2189](https://github.com/zeta-chain/node/pull/2189) - Updated the docker tag when a release trigger runs to be the github event for the release name which should be the version. Removed mac specific build as the arm build should handle that. diff --git a/testutil/sample/crosschain.go b/testutil/sample/crosschain.go index 5e9a1f952c..f5c9dd32fb 100644 --- a/testutil/sample/crosschain.go +++ b/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) diff --git a/x/crosschain/keeper/grpc_query_cctx_rate_limit.go b/x/crosschain/keeper/grpc_query_cctx_rate_limit.go index e28f3e3677..270fb10b87 100644 --- a/x/crosschain/keeper/grpc_query_cctx_rate_limit.go +++ b/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,7 +331,8 @@ 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 @@ -325,8 +340,8 @@ func (k Keeper) ListPendingCctxWithinRateLimit( 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, 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 07f2db200b..409031e121 100644 --- a/x/crosschain/keeper/grpc_query_cctx_rate_limit_test.go +++ b/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, "", @@ -124,12 +128,39 @@ func TestKeeper_RateLimiterInput(t *testing.T) { types.CctxStatus_PendingOutbound, ) + // create Eth chain 999 reverted and 200 pending revert 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 + // these cctxs should be ignored by the rate limiter as it can't compare `ObservedExternalHeight` against the 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, "", @@ -140,6 +171,7 @@ func TestKeeper_RateLimiterInput(t *testing.T) { t, 1000, 1199, + zetaChainID, btcChainID, coin.CoinType_Gas, "", @@ -216,7 +248,54 @@ func TestKeeper_RateLimiterInput(t *testing.T) { ), expectedTotalPending: 400, expectedPastCctxsValue: sdk.NewInt(1200).Mul(sdk.NewInt(1e18)).String(), // 400 * (2.5 + 0.5) ZETA - expectedPendingCctxsValue: sdk.NewInt(300).Mul(sdk.NewInt(1e18)).String(), // 100 * 1e15 ZETA + expectedPendingCctxsValue: sdk.NewInt(300).Mul(sdk.NewInt(1e18)).String(), // 100 * (2.5 + 0.5) ZETA + expectedLowestPendingCctxHeight: 1100, + }, + { + name: "scan retrieve all pending cctxs and ignore revert cctxs", + rateLimitFlags: createTestRateLimiterFlags( + 500, + math.NewUint(10*1e18), + zrc20ETH, + zrc20BTC, + zrc20USDT, + "2500", + "50000", + "0.8", + ), + ethMinedCctxs: ethRevertedCctxs, + ethPendingCctxs: ethPendingRevertCctxs, + 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: 0, // use default MaxPendingCctxs + + // expected results + expectedHeight: 1199, + expectedCctxsMissed: keeper.SortCctxsByHeightAndChainID( + append(append([]*types.CrossChainTx{}, ethPendingRevertCctxs[0:100]...), btcPendingCctxs[0:100]...), + ), + expectedCctxsPending: keeper.SortCctxsByHeightAndChainID( + append(append([]*types.CrossChainTx{}, ethPendingRevertCctxs[100:200]...), btcPendingCctxs[100:200]...), + ), + expectedTotalPending: 400, + expectedPastCctxsValue: sdk.NewInt(200). + Mul(sdk.NewInt(1e18)). + String(), + // 400 * 0.5 ZETA, ignore Eth chain reverted cctxs + expectedPendingCctxsValue: sdk.NewInt(300).Mul(sdk.NewInt(1e18)).String(), // 100 * (2.5 + 0.5) ZETA expectedLowestPendingCctxHeight: 1100, }, { @@ -260,7 +339,7 @@ func TestKeeper_RateLimiterInput(t *testing.T) { ), expectedTotalPending: 400, expectedPastCctxsValue: sdk.NewInt(3297).Mul(sdk.NewInt(1e18)).String(), // 1099 * (2.5 + 0.5) ZETA - expectedPendingCctxsValue: sdk.NewInt(300).Mul(sdk.NewInt(1e18)).String(), // 100 * 1e15 ZETA + expectedPendingCctxsValue: sdk.NewInt(300).Mul(sdk.NewInt(1e18)).String(), // 100 * (2.5 + 0.5) ZETA expectedLowestPendingCctxHeight: 1100, }, { @@ -470,6 +549,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 +562,7 @@ func TestKeeper_ListPendingCctxWithinRateLimit(t *testing.T) { t, 1, 999, + zetaChainID, ethChainID, coin.CoinType_Gas, "", @@ -492,6 +573,7 @@ func TestKeeper_ListPendingCctxWithinRateLimit(t *testing.T) { t, 1000, 1199, + zetaChainID, ethChainID, coin.CoinType_Gas, "", @@ -499,12 +581,38 @@ func TestKeeper_ListPendingCctxWithinRateLimit(t *testing.T) { 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 +623,7 @@ func TestKeeper_ListPendingCctxWithinRateLimit(t *testing.T) { t, 1000, 1199, + zetaChainID, btcChainID, coin.CoinType_Gas, "", @@ -644,6 +753,44 @@ 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( diff --git a/zetaclient/orchestrator/orchestrator_test.go b/zetaclient/orchestrator/orchestrator_test.go index f4dcf1ffd0..c47c3c0bb1 100644 --- a/zetaclient/orchestrator/orchestrator_test.go +++ b/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, "", diff --git a/zetaclient/ratelimiter/rate_limiter_test.go b/zetaclient/ratelimiter/rate_limiter_test.go index c028b3c526..0ce936bb72 100644 --- a/zetaclient/ratelimiter/rate_limiter_test.go +++ b/zetaclient/ratelimiter/rate_limiter_test.go @@ -115,6 +115,7 @@ func Test_ApplyRateLimiter(t *testing.T) { // define test chain ids ethChainID := chains.EthChain.ChainId btcChainID := chains.BtcMainnetChain.ChainId + zetaChainID := chains.ZetaChainMainnet.ChainId // create 10 missed and 90 pending cctxs for eth chain, the coinType/amount does not matter for this test // but we still use a proper cctx value (0.5 ZETA) to make the test more realistic @@ -122,6 +123,7 @@ func Test_ApplyRateLimiter(t *testing.T) { t, 1, 10, + zetaChainID, ethChainID, coin.CoinType_Gas, "", @@ -132,6 +134,7 @@ func Test_ApplyRateLimiter(t *testing.T) { t, 11, 100, + zetaChainID, ethChainID, coin.CoinType_Gas, "", @@ -146,6 +149,7 @@ func Test_ApplyRateLimiter(t *testing.T) { t, 1, 10, + zetaChainID, btcChainID, coin.CoinType_Gas, "", @@ -156,6 +160,7 @@ func Test_ApplyRateLimiter(t *testing.T) { t, 11, 100, + zetaChainID, btcChainID, coin.CoinType_Gas, "",