From 6a9acb250bf701ca1e0b480e0bdb00e06089cc28 Mon Sep 17 00:00:00 2001 From: Logan Nguyen Date: Sat, 16 Nov 2024 17:25:46 -0500 Subject: [PATCH] chore: cherry pick #3252 to release/0.60 (#3269) fix: Allow the HBar Rate Limiter to be disabled. (#3252) * fix: Allow the HBar Rate Limiter to be disabled. * fix: Added acceptance test. * fix: Improved test by adding condition that would normally trigger rate limiting, and added isEnabled() method. * fix: Cleaned up and tightened test. * fix: Updated workflow for new tests. * fix: Updated comment. * fix: added the new hbarlimiter_batch_3 to the public_result in the workflow * fix: Test clean up. fix: clean up of experimental code. * fix: added isEanbled check to addExpense. * fix: Check totalBudget instead of remainingBudget in constructor. * fix: clean up. * fix: Test fix. Now that the addExpense is also skipped when the HBar Rate Limiter is disabled the test should not use the expenses aggregated to determine if the maxSpendingLimit has been passed, but simply use the relay operator's before and after balances. * fix: Added note around nullish coalescing operator. * fix: Added check for remaining budget at start of test and renamed flag to more meaningful name. --------- Signed-off-by: Eric Badiere Co-authored-by: Eric Badiere --- .github/workflows/acceptance.yml | 7 ++ package.json | 1 + packages/relay/src/lib/constants.ts | 4 +- .../hbarLimitService/IHbarLimitService.ts | 1 + .../lib/services/hbarLimitService/index.ts | 26 ++++++ .../hbarLimitService/hbarLimitService.spec.ts | 40 ++++++++ .../tests/acceptance/hbarLimiter.spec.ts | 92 +++++++++++++------ 7 files changed, 143 insertions(+), 28 deletions(-) diff --git a/.github/workflows/acceptance.yml b/.github/workflows/acceptance.yml index acff2a981..784878113 100644 --- a/.github/workflows/acceptance.yml +++ b/.github/workflows/acceptance.yml @@ -55,6 +55,12 @@ jobs: with: testfilter: hbarlimiter_batch2 + hbarlimiter_batch_3: + name: HBar Limiter Batch 3 + uses: ./.github/workflows/acceptance-workflow.yml + with: + testfilter: hbarlimiter_batch3 + tokencreate: name: Token Create uses: ./.github/workflows/acceptance-workflow.yml @@ -123,6 +129,7 @@ jobs: - ratelimiter - hbarlimiter_batch_1 - hbarlimiter_batch_2 + - hbarlimiter_batch_3 - tokencreate - tokenmanagement - htsprecompilev1 diff --git a/package.json b/package.json index 7eec7fd11..b0df177bc 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,7 @@ "acceptancetest:ratelimiter": "nyc ts-mocha packages/ws-server/tests/acceptance/index.spec.ts -g '@web-socket-ratelimiter' --exit && ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@ratelimiter' --exit", "acceptancetest:hbarlimiter_batch1": "HBAR_RATE_LIMIT_BASIC=1000000000 HBAR_RATE_LIMIT_EXTENDED=1500000000 HBAR_RATE_LIMIT_PRIVILEGED=2000000000 nyc ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@hbarlimiter-batch1' --exit", "acceptancetest:hbarlimiter_batch2": "HBAR_RATE_LIMIT_BASIC=1000000000 HBAR_RATE_LIMIT_EXTENDED=1500000000 HBAR_RATE_LIMIT_PRIVILEGED=2000000000 nyc ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@hbarlimiter-batch2' --exit", + "acceptancetest:hbarlimiter_batch3": "HBAR_RATE_LIMIT_TINYBAR=0 HBAR_RATE_LIMIT_BASIC=1000000000 HBAR_RATE_LIMIT_EXTENDED=1500000000 HBAR_RATE_LIMIT_PRIVILEGED=2000000000 nyc ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@hbarlimiter-batch3' --exit", "acceptancetest:tokencreate": "nyc ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@tokencreate' --exit", "acceptancetest:tokenmanagement": "nyc ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@tokenmanagement' --exit", "acceptancetest:htsprecompilev1": "nyc ts-mocha packages/server/tests/acceptance/index.spec.ts -g '@htsprecompilev1' --exit", diff --git a/packages/relay/src/lib/constants.ts b/packages/relay/src/lib/constants.ts index 305da0e2d..8c6b9eaeb 100644 --- a/packages/relay/src/lib/constants.ts +++ b/packages/relay/src/lib/constants.ts @@ -145,7 +145,9 @@ export default { // @ts-ignore HBAR_RATE_LIMIT_DURATION: parseInt(ConfigService.get('HBAR_RATE_LIMIT_DURATION') || '86400000'), // 1 day // @ts-ignore - HBAR_RATE_LIMIT_TOTAL: BigNumber(ConfigService.get('HBAR_RATE_LIMIT_TINYBAR') || '800000000000'), // 8000 HBARs + // The logical OR operator || returns the first truthy value and 0 is falsy. + // The nullish coalescing operator ?? falls back to the default value when the left-hand operand is null or undefined, not when it's 0 or any other falsy value. + HBAR_RATE_LIMIT_TOTAL: BigNumber(ConfigService.get('HBAR_RATE_LIMIT_TINYBAR') ?? '800000000000'), // 8000 HBARs // @ts-ignore HBAR_RATE_LIMIT_BASIC: BigNumber(ConfigService.get('HBAR_RATE_LIMIT_BASIC') || '1120000000'), // 11.2 HBARs // @ts-ignore diff --git a/packages/relay/src/lib/services/hbarLimitService/IHbarLimitService.ts b/packages/relay/src/lib/services/hbarLimitService/IHbarLimitService.ts index cc7ffc9c4..0018024e6 100644 --- a/packages/relay/src/lib/services/hbarLimitService/IHbarLimitService.ts +++ b/packages/relay/src/lib/services/hbarLimitService/IHbarLimitService.ts @@ -31,4 +31,5 @@ export interface IHbarLimitService { estimatedTxFee?: number, ): Promise; addExpense(cost: number, ethAddress: string, requestDetails: RequestDetails, ipAddress?: string): Promise; + isEnabled(): boolean; } diff --git a/packages/relay/src/lib/services/hbarLimitService/index.ts b/packages/relay/src/lib/services/hbarLimitService/index.ts index 5e4b97a6b..0acea7276 100644 --- a/packages/relay/src/lib/services/hbarLimitService/index.ts +++ b/packages/relay/src/lib/services/hbarLimitService/index.ts @@ -37,6 +37,12 @@ export class HbarLimitService implements IHbarLimitService { PRIVILEGED: Hbar.fromTinybars(constants.HBAR_RATE_LIMIT_PRIVILEGED), }; + /** + * Flag to turn off the HBarRateLimitService. + * @private + */ + private readonly isHBarRateLimiterEnabled: boolean = true; + /** * Counts the number of times the rate limit has been reached. * @private @@ -90,6 +96,10 @@ export class HbarLimitService implements IHbarLimitService { this.reset = this.getResetTimestamp(); this.remainingBudget = this.totalBudget; + if (this.totalBudget.toTinybars().lte(0)) { + this.isHBarRateLimiterEnabled = false; + } + const metricCounterName = 'rpc_relay_hbar_rate_limit'; this.register.removeSingleMetric(metricCounterName); this.hbarLimitCounter = new Counter({ @@ -142,6 +152,14 @@ export class HbarLimitService implements IHbarLimitService { ); } + /** + * Checks if the rate limiter is enabled. + * returns {boolean} - `true` if the rate limiter is enabled, otherwise `false`. + */ + isEnabled(): boolean { + return this.isHBarRateLimiterEnabled; + } + /** * Resets the {@link HbarSpendingPlan#amountSpent} field for all existing plans. * @param {RequestDetails} requestDetails - The request details used for logging and tracking. @@ -181,6 +199,10 @@ export class HbarLimitService implements IHbarLimitService { requestDetails: RequestDetails, estimatedTxFee: number = 0, ): Promise { + if (!this.isEnabled()) { + return false; + } + const ipAddress = requestDetails.ipAddress; if (await this.isTotalBudgetExceeded(mode, methodName, txConstructorName, estimatedTxFee, requestDetails)) { return true; @@ -235,6 +257,10 @@ export class HbarLimitService implements IHbarLimitService { * @returns {Promise} - A promise that resolves when the expense has been added. */ async addExpense(cost: number, ethAddress: string, requestDetails: RequestDetails): Promise { + if (!this.isEnabled()) { + return; + } + const newRemainingBudget = this.remainingBudget.toTinybars().sub(cost); this.remainingBudget = Hbar.fromTinybars(newRemainingBudget); this.hbarLimitRemainingGauge.set(newRemainingBudget.toNumber()); diff --git a/packages/relay/tests/lib/services/hbarLimitService/hbarLimitService.spec.ts b/packages/relay/tests/lib/services/hbarLimitService/hbarLimitService.spec.ts index 06ff203d5..a1da7c748 100644 --- a/packages/relay/tests/lib/services/hbarLimitService/hbarLimitService.spec.ts +++ b/packages/relay/tests/lib/services/hbarLimitService/hbarLimitService.spec.ts @@ -494,6 +494,46 @@ describe('HBAR Rate Limit Service', function () { expect(result).to.be.false; }); }); + + describe('disable the rate limiter', function () { + const hbarLimitServiceDisabled = new HbarLimitService( + hbarSpendingPlanRepositoryStub, + ethAddressHbarSpendingPlanRepositoryStub, + ipAddressHbarSpendingPlanRepositoryStub, + logger, + register, + Hbar.fromTinybars(0), + limitDuration, + ); + + it('should return false if the rate limiter is disabled by setting HBAR_RATE_LIMIT_TINYBAR to zero', async function () { + const spendingPlan = createSpendingPlan( + mockPlanId, + HbarLimitService.TIER_LIMITS[SubscriptionTier.BASIC].toTinybars().sub(mockEstimatedTxFee).add(1), + ); + ethAddressHbarSpendingPlanRepositoryStub.findByAddress.resolves({ + ethAddress: mockEthAddress, + planId: mockPlanId, + }); + hbarSpendingPlanRepositoryStub.findByIdWithDetails.resolves(spendingPlan); + + const result = await hbarLimitServiceDisabled.shouldLimit( + mode, + methodName, + txConstructorName, + mockEthAddress, + requestDetails, + mockEstimatedTxFee, + ); + + // Rate limiter is disabled, so it should return false + expect(result).to.be.false; + }); + + it('should return undefined if the rate limiter is disabled and addExpense is called', async function () { + expect(await hbarLimitServiceDisabled.addExpense(100, mockEthAddress, requestDetails)).to.be.undefined; + }); + }); }); describe('getSpendingPlan', function () { diff --git a/packages/server/tests/acceptance/hbarLimiter.spec.ts b/packages/server/tests/acceptance/hbarLimiter.spec.ts index 002dc2fa1..d7d3e3d27 100644 --- a/packages/server/tests/acceptance/hbarLimiter.spec.ts +++ b/packages/server/tests/acceptance/hbarLimiter.spec.ts @@ -91,6 +91,33 @@ describe('@hbarlimiter HBAR Limiter Acceptance Tests', function () { logger.child({ name: 'hbar-spending-plan-repository' }), ); + const pollForProperAmountSpent = async ( + hbarSpendingPlan: IDetailedHbarSpendingPlan, + deploymentCounts: number, + expectedTxCost: number, + ) => { + let amountSpent = (await hbarSpendingPlanRepository.findByIdWithDetails(hbarSpendingPlan.id, requestDetails)) + .amountSpent; + + while (amountSpent < deploymentCounts * expectedTxCost) { + logger.warn( + `Fail to retrieve proper amount spent by the spending plan. Polling for the proper amount: deploymentCounts=${deploymentCounts}, expectedTxCost=${expectedTxCost}, amountSpent=${amountSpent}, properAmountSpent=${ + deploymentCounts * expectedTxCost + }, planId=${hbarSpendingPlan.id}`, + ); + await Utils.wait(3000); + amountSpent = (await hbarSpendingPlanRepository.findByIdWithDetails(hbarSpendingPlan.id, requestDetails)) + .amountSpent; + } + + logger.info( + `Successfully retrieve proper amount spent by hbarSpendingPlan: deploymentCounts=${deploymentCounts}, expectedTxCost=${expectedTxCost}, amountSpent=${amountSpent}, properAmountSpent=${ + deploymentCounts * expectedTxCost + }, planId=${hbarSpendingPlan.id}`, + ); + return amountSpent; + }; + // The following tests exhaust the hbar limit, so they should only be run against a local relay if (relayIsLocal) { const deployContract = async (contractJson: any, wallet: ethers.Wallet): Promise => { @@ -414,33 +441,6 @@ describe('@hbarlimiter HBAR Limiter Acceptance Tests', function () { return { aliasAccounts, hbarSpendingPlan }; }; - const pollForProperAmountSpent = async ( - hbarSpendingPlan: IDetailedHbarSpendingPlan, - deploymentCounts: number, - expectedTxCost: number, - ) => { - let amountSpent = (await hbarSpendingPlanRepository.findByIdWithDetails(hbarSpendingPlan.id, requestDetails)) - .amountSpent; - - while (amountSpent < deploymentCounts * expectedTxCost) { - logger.warn( - `Fail to retrieve proper amount spent by the spending plan. Polling for the proper amount: deploymentCounts=${deploymentCounts}, expectedTxCost=${expectedTxCost}, amountSpent=${amountSpent}, properAmountSpent=${ - deploymentCounts * expectedTxCost - }, planId=${hbarSpendingPlan.id}`, - ); - await Utils.wait(3000); - amountSpent = (await hbarSpendingPlanRepository.findByIdWithDetails(hbarSpendingPlan.id, requestDetails)) - .amountSpent; - } - - logger.info( - `Successfully retrieve proper amount spent by hbarSpendingPlan: deploymentCounts=${deploymentCounts}, expectedTxCost=${expectedTxCost}, amountSpent=${amountSpent}, properAmountSpent=${ - deploymentCounts * expectedTxCost - }, planId=${hbarSpendingPlan.id}`, - ); - return amountSpent; - }; - describe('@hbarlimiter-batch1 BASIC Tier', () => { beforeEach(async function () { const basicPlans = await hbarSpendingPlanRepository.findAllActiveBySubscriptionTier( @@ -887,5 +887,43 @@ describe('@hbarlimiter HBAR Limiter Acceptance Tests', function () { }); }); }); + + describe('@hbarlimiter-batch3 Unlimited', () => { + before(async function () { + logger.info( + `${requestDetails.formattedRequestId} HBAR_RATE_LIMIT_TINYBAR: ${ConfigService.get( + 'HBAR_RATE_LIMIT_TINYBAR', + )}`, + ); + }); + + it('should eventually exhaust the hbar limit for a BASIC user after multiple deployments of large contracts, and not throw an error', async function () { + // confirm that HBAR_RATE_LIMIT_TINYBAR is set to zero + expect(ConfigService.get('HBAR_RATE_LIMIT_TINYBAR')).to.eq(0); + // This should set the remaining HBAR limit to zero + const remainingHbarsBefore = Number(await metrics.get(testConstants.METRICS.REMAINING_HBAR_LIMIT)); + expect(remainingHbarsBefore).to.eq(0); + let deploymentCounts = 0; + + const operatorBalanceBefore = (await mirrorNode.get(`/accounts/${operatorAccount}`, requestId)).balance.balance; + + for (deploymentCounts = 0; deploymentCounts < 3; deploymentCounts++) { + const tx = await deployContract(largeContractJson, global.accounts[0].wallet); + await tx.waitForDeployment(); + } + // Verify that the amount spent exceeds the HBAR limit + const operatorBalanceAfter = (await mirrorNode.get(`/accounts/${operatorAccount}`, requestId)).balance.balance; + const amountSpent = operatorBalanceBefore - operatorBalanceAfter; + expect(amountSpent).to.be.gt(maxBasicSpendingLimit); + + // Verify that remaining HBAR limit is zero + const remainingHbarsAfter = Number(await metrics.get(testConstants.METRICS.REMAINING_HBAR_LIMIT)); + expect(remainingHbarsAfter).to.be.eq(0); + + // Confirm that deployments continued even after the limit was exhausted + expect(deploymentCounts).to.be.gte(1); + await expect(deployContract(largeContractJson, global.accounts[0].wallet)).to.be.fulfilled; + }); + }); } });