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 <ebadiere@gmail.com> Co-authored-by: Eric Badiere <ebadiere@gmail.com>
hashgraph · Nov 16, 2024 · 6a9acb2 · 6a9acb2
1 parent 8648335
commit 6a9acb2
Show file tree

Hide file tree

Showing 7 changed files with 143 additions and 28 deletions.
diff --git 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
@@ -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
@@ -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
@@ -31,4 +31,5 @@ export interface IHbarLimitService {
     estimatedTxFee?: number,
   ): Promise<boolean>;
   addExpense(cost: number, ethAddress: string, requestDetails: RequestDetails, ipAddress?: string): Promise<void>;
+  isEnabled(): boolean;
 }
diff --git 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<boolean> {
+    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<void>} - A promise that resolves when the expense has been added.
    */
   async addExpense(cost: number, ethAddress: string, requestDetails: RequestDetails): Promise<void> {
+    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
@@ -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
@@ -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<ethers.Contract> => {
@@ -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;
+      });
+    });
   }
 });