From b3f772fc2632882e5ea308819b476ca979ef3d13 Mon Sep 17 00:00:00 2001 From: Victor Yanev Date: Fri, 11 Oct 2024 18:25:48 +0300 Subject: [PATCH 1/2] fix: bugged metrics and wrong env vars Signed-off-by: Victor Yanev --- packages/relay/src/lib/constants.ts | 6 +- .../lib/services/hbarLimitService/index.ts | 71 ++++++++++--------- .../hbarLimitService/hbarLimitService.spec.ts | 44 ++++++------ 3 files changed, 63 insertions(+), 58 deletions(-) diff --git a/packages/relay/src/lib/constants.ts b/packages/relay/src/lib/constants.ts index abcbf5e6af..4c1b7859c1 100644 --- a/packages/relay/src/lib/constants.ts +++ b/packages/relay/src/lib/constants.ts @@ -142,9 +142,9 @@ export default { // TODO: Replace with actual values - https://github.com/hashgraph/hedera-json-rpc-relay/issues/2895 HBAR_RATE_LIMIT_DURATION: parseInt(process.env.HBAR_RATE_LIMIT_DURATION || '80000'), // 80 seconds HBAR_RATE_LIMIT_TOTAL: BigNumber(process.env.HBAR_RATE_LIMIT_TINYBAR || '11000000000'), // 110 HBARs per 80 seconds - HBAR_RATE_LIMIT_BASIC: BigNumber(process.env.HBAR_DAILY_LIMIT_BASIC || '92592592'), // Equivalent of 1000 HBARs per day - HBAR_RATE_LIMIT_EXTENDED: BigNumber(process.env.HBAR_DAILY_LIMIT_EXTENDED || '925925925'), // Equivalent of 10000 HBARs per day - HBAR_RATE_LIMIT_PRIVILEGED: BigNumber(process.env.HBAR_DAILY_LIMIT_PRIVILEGED || '1851851850'), // Equivalent of 20000 HBARs per day + HBAR_RATE_LIMIT_BASIC: BigNumber(process.env.HBAR_RATE_LIMIT_BASIC || '92592592'), // Equivalent of 1000 HBARs per day + HBAR_RATE_LIMIT_EXTENDED: BigNumber(process.env.HBAR_RATE_LIMIT_EXTENDED || '925925925'), // Equivalent of 10000 HBARs per day + HBAR_RATE_LIMIT_PRIVILEGED: BigNumber(process.env.HBAR_RATE_LIMIT_PRIVILEGED || '1851851850'), // Equivalent of 20000 HBARs per day GAS_PRICE_TINY_BAR_BUFFER: parseInt(process.env.GAS_PRICE_TINY_BAR_BUFFER || '10000000000'), WEB_SOCKET_PORT: process.env.WEB_SOCKET_PORT || 8546, WEB_SOCKET_HTTP_PORT: process.env.WEB_SOCKET_HTTP_PORT || 8547, diff --git a/packages/relay/src/lib/services/hbarLimitService/index.ts b/packages/relay/src/lib/services/hbarLimitService/index.ts index 7cd11af069..cffe64c7d4 100644 --- a/packages/relay/src/lib/services/hbarLimitService/index.ts +++ b/packages/relay/src/lib/services/hbarLimitService/index.ts @@ -50,21 +50,17 @@ export class HbarLimitService implements IHbarLimitService { private readonly hbarLimitRemainingGauge: Gauge; /** - * Tracks the number of unique spending plans that have been utilized on a daily basis - * (i.e., plans that had expenses added to them). - * - * For basic spending plans, this equates to the number of unique users who have made requests on that day, - * since each user has their own individual spending plan. + * Tracks the number of unique ETH addresses that have made requests. * * @private */ - private readonly dailyUniqueSpendingPlansCounter: Record; + private readonly uniqueSpendingPlansCounter: Record; /** - * Tracks the average daily spending plan usages. + * Tracks the average amount of tinybars spent by spending plans per subscription tier * @private */ - private readonly averageDailySpendingPlanUsagesGauge: Record; + private readonly averageSpendingPlanAmountSpentGauge: Record; /** * The remaining budget for the rate limiter. @@ -109,13 +105,13 @@ export class HbarLimitService implements IHbarLimitService { }); this.hbarLimitRemainingGauge.set(this.remainingBudget.toTinybars().toNumber()); - this.dailyUniqueSpendingPlansCounter = Object.values(SubscriptionTier).reduce( - (acc, type) => { - const dailyUniqueSpendingPlansCounterName = `daily_unique_spending_plans_counter_${type.toLowerCase()}`; - this.register.removeSingleMetric(dailyUniqueSpendingPlansCounterName); - acc[type] = new Counter({ - name: dailyUniqueSpendingPlansCounterName, - help: `Tracks the number of unique spending plans used daily for ${type} subscription tier`, + this.uniqueSpendingPlansCounter = Object.values(SubscriptionTier).reduce( + (acc, tier) => { + const uniqueSpendingPlansCounterName = `unique_spending_plans_counter_${tier.toLowerCase()}`; + this.register.removeSingleMetric(uniqueSpendingPlansCounterName); + acc[tier] = new Counter({ + name: uniqueSpendingPlansCounterName, + help: `Tracks the number of unique ${tier} spending plans used during the limit duration`, registers: [register], }); return acc; @@ -123,13 +119,13 @@ export class HbarLimitService implements IHbarLimitService { {} as Record, ); - this.averageDailySpendingPlanUsagesGauge = Object.values(SubscriptionTier).reduce( - (acc, type) => { - const averageDailySpendingGaugeName = `average_daily_spending_plan_usages_gauge_${type.toLowerCase()}`; - this.register.removeSingleMetric(averageDailySpendingGaugeName); - acc[type] = new Gauge({ - name: averageDailySpendingGaugeName, - help: `Tracks the average daily spending plan usages for ${type} subscription tier`, + this.averageSpendingPlanAmountSpentGauge = Object.values(SubscriptionTier).reduce( + (acc, tier) => { + const averageAmountSpentGaugeName = `average_spending_plan_amount_spent_gauge_${tier.toLowerCase()}`; + this.register.removeSingleMetric(averageAmountSpentGaugeName); + acc[tier] = new Gauge({ + name: averageAmountSpentGaugeName, + help: `Tracks the average amount of tinybars spent by ${tier} spending plans`, registers: [register], }); return acc; @@ -147,6 +143,7 @@ export class HbarLimitService implements IHbarLimitService { this.logger.trace(`${requestDetails.formattedRequestId} Resetting HBAR rate limiter...`); await this.hbarSpendingPlanRepository.resetAmountSpentOfAllPlans(requestDetails); this.resetBudget(); + this.resetTemporaryMetrics(); this.reset = this.getResetTimestamp(); this.logger.trace( `${requestDetails.formattedRequestId} HBAR Rate Limit reset: remainingBudget=${this.remainingBudget}, newResetTimestamp=${this.reset}`, @@ -170,7 +167,7 @@ export class HbarLimitService implements IHbarLimitService { estimatedTxFee: number = 0, ): Promise { const ipAddress = requestDetails.ipAddress; - if (await this.isDailyBudgetExceeded(mode, methodName, estimatedTxFee, requestDetails)) { + if (await this.isTotalBudgetExceeded(mode, methodName, estimatedTxFee, requestDetails)) { return true; } if (!ethAddress && !ipAddress) { @@ -224,7 +221,7 @@ export class HbarLimitService implements IHbarLimitService { // Check if the spending plan is being used for the first time today if (spendingPlan.amountSpent === 0) { - this.dailyUniqueSpendingPlansCounter[spendingPlan.subscriptionTier].inc(1); + this.uniqueSpendingPlansCounter[spendingPlan.subscriptionTier].inc(1); } await this.hbarSpendingPlanRepository.addToAmountSpent(spendingPlan.id, cost, requestDetails, this.limitDuration); @@ -232,7 +229,7 @@ export class HbarLimitService implements IHbarLimitService { this.hbarLimitRemainingGauge.set(this.remainingBudget.toTinybars().toNumber()); // Done asynchronously in the background - this.updateAverageDailyUsagePerSubscriptionTier(spendingPlan.subscriptionTier, requestDetails).then(); + this.updateAverageAmountSpentPerSubscriptionTier(spendingPlan.subscriptionTier, requestDetails).then(); this.logger.trace( `${requestDetails.formattedRequestId} HBAR rate limit expense update: cost=${cost} tℏ, remainingBudget=${this.remainingBudget}`, @@ -240,15 +237,15 @@ export class HbarLimitService implements IHbarLimitService { } /** - * Checks if the total daily budget has been exceeded. + * Checks if the total budget of the limiter has been exceeded. * @param {string} mode - The mode of the transaction or request. * @param {string} methodName - The name of the method being invoked. * @param {number} estimatedTxFee - The total estimated transaction fee, default to 0. * @param {RequestDetails} requestDetails The request details for logging and tracking - * @returns {Promise} - Resolves `true` if the daily budget has been exceeded, otherwise `false`. + * @returns {Promise} - Resolves `true` if the total budget has been exceeded, otherwise `false`. * @private */ - private async isDailyBudgetExceeded( + private async isTotalBudgetExceeded( mode: string, methodName: string, estimatedTxFee: number = 0, @@ -272,12 +269,12 @@ export class HbarLimitService implements IHbarLimitService { } /** - * Updates the average daily usage per subscription tier. - * @param {SubscriptionTier} subscriptionTier - The subscription tier to update the average daily usage for. + * Updates the average amount of tinybars spent of spending plans per subscription tier. + * @param {SubscriptionTier} subscriptionTier - The subscription tier to update the average usage for. * @param {RequestDetails} requestDetails - The request details for logging and tracking. - * @private {Promise} - A promise that resolves when the average daily usage has been updated. + * @private {Promise} - A promise that resolves when the average usage has been updated. */ - private async updateAverageDailyUsagePerSubscriptionTier( + private async updateAverageAmountSpentPerSubscriptionTier( subscriptionTier: SubscriptionTier, requestDetails: RequestDetails, ): Promise { @@ -287,7 +284,7 @@ export class HbarLimitService implements IHbarLimitService { ); const totalUsage = plans.reduce((total, plan) => total + plan.amountSpent, 0); const averageUsage = Math.round(totalUsage / plans.length); - this.averageDailySpendingPlanUsagesGauge[subscriptionTier].set(averageUsage); + this.averageSpendingPlanAmountSpentGauge[subscriptionTier].set(averageUsage); } /** @@ -308,6 +305,14 @@ export class HbarLimitService implements IHbarLimitService { this.hbarLimitRemainingGauge.set(this.remainingBudget.toTinybars().toNumber()); } + /** + * Resets the metrics which are used to track the number of unique spending plans used during the limit duration. + * @private + */ + private resetTemporaryMetrics(): void { + Object.values(SubscriptionTier).forEach((tier) => this.uniqueSpendingPlansCounter[tier].reset()); + } + /** * Calculates the next reset timestamp for the rate limiter. * diff --git a/packages/relay/tests/lib/services/hbarLimitService/hbarLimitService.spec.ts b/packages/relay/tests/lib/services/hbarLimitService/hbarLimitService.spec.ts index 3785e85788..e9291a315c 100644 --- a/packages/relay/tests/lib/services/hbarLimitService/hbarLimitService.spec.ts +++ b/packages/relay/tests/lib/services/hbarLimitService/hbarLimitService.spec.ts @@ -100,8 +100,8 @@ describe('HbarLimitService', function () { expect(hbarLimitService['hbarLimitCounter']).to.be.instanceOf(Counter); expect(hbarLimitService['hbarLimitRemainingGauge']).to.be.instanceOf(Gauge); Object.values(SubscriptionTier).forEach((tier) => { - expect(hbarLimitService['dailyUniqueSpendingPlansCounter'][tier]).to.be.instanceOf(Counter); - expect(hbarLimitService['averageDailySpendingPlanUsagesGauge'][tier]).to.be.instanceOf(Gauge); + expect(hbarLimitService['uniqueSpendingPlansCounter'][tier]).to.be.instanceOf(Counter); + expect(hbarLimitService['averageSpendingPlanAmountSpentGauge'][tier]).to.be.instanceOf(Gauge); }); }); @@ -175,7 +175,7 @@ describe('HbarLimitService', function () { describe('shouldLimit', function () { describe('based on ethAddress', async function () { - it('should return true if the total daily budget is exceeded', async function () { + it('should return true if the total budget is exceeded', async function () { // @ts-ignore hbarLimitService.remainingBudget = Hbar.fromTinybars(0); const result = await hbarLimitService.shouldLimit(mode, methodName, mockEthAddress, requestDetails); @@ -322,7 +322,7 @@ describe('HbarLimitService', function () { }); describe('based on ipAddress', async function () { - it('should return true if the total daily budget is exceeded', async function () { + it('should return true if the total budget is exceeded', async function () { // @ts-ignore hbarLimitService.remainingBudget = Hbar.fromTinybars(0); const result = await hbarLimitService.shouldLimit(mode, methodName, '', requestDetails); @@ -627,17 +627,17 @@ describe('HbarLimitService', function () { spendingHistory: [{ amount: expense, timestamp: new Date() }], }, ]); - const incDailyUniqueSpendingPlansCounterSpy = sinon.spy( - hbarLimitService['dailyUniqueSpendingPlansCounter'][SubscriptionTier.BASIC], + const incUniqueSpendingPlansCounterSpy = sinon.spy( + hbarLimitService['uniqueSpendingPlansCounter'][SubscriptionTier.BASIC], 'inc', ); - const setAverageDailySpendingPlanUsagesGaugeSpy = sinon.spy( - hbarLimitService['averageDailySpendingPlanUsagesGauge'][SubscriptionTier.BASIC], + const setAverageSpendingPlanAmountSpentGaugeSpy = sinon.spy( + hbarLimitService['averageSpendingPlanAmountSpentGauge'][SubscriptionTier.BASIC], 'set', ); - const updateAverageDailyUsagePerSubscriptionTierSpy = sinon.spy( + const updateAverageAmountSpentPerSubscriptionTierSpy = sinon.spy( hbarLimitService, - 'updateAverageDailyUsagePerSubscriptionTier', + 'updateAverageAmountSpentPerSubscriptionTier' as any, ); await hbarLimitService.addExpense(expense, ethAddress, requestDetails); @@ -649,10 +649,10 @@ describe('HbarLimitService', function () { expect((await hbarLimitService['hbarLimitRemainingGauge'].get()).values[0].value).to.equal( hbarLimitService['totalBudget'].toTinybars().sub(expense).toNumber(), ); - await Promise.all(updateAverageDailyUsagePerSubscriptionTierSpy.returnValues); + await Promise.all(updateAverageAmountSpentPerSubscriptionTierSpy.returnValues); const expectedAverageUsage = Math.round((otherPlanOfTheSameTier.amountSpent + expense) / 2); - sinon.assert.calledOnceWithExactly(setAverageDailySpendingPlanUsagesGaugeSpy, expectedAverageUsage); - sinon.assert.calledOnceWithExactly(incDailyUniqueSpendingPlansCounterSpy, 1); + sinon.assert.calledOnceWithExactly(setAverageSpendingPlanAmountSpentGaugeSpy, expectedAverageUsage); + sinon.assert.calledOnceWithExactly(incUniqueSpendingPlansCounterSpy, 1); }; it('should throw an error if empty ethAddress or ipAddress is provided', async function () { @@ -710,31 +710,31 @@ describe('HbarLimitService', function () { }); }); - describe('isDailyBudgetExceeded', function () { - const testIsDailyBudgetExceeded = async (remainingBudget: number, expected: boolean) => { + describe('isTotalBudgetExceeded', function () { + const testIsTotalBudgetExceeded = async (remainingBudget: number, expected: boolean) => { // @ts-ignore hbarLimitService.remainingBudget = Hbar.fromTinybars(remainingBudget); await expect( - hbarLimitService['isDailyBudgetExceeded'](mode, methodName, undefined, requestDetails), + hbarLimitService['isTotalBudgetExceeded'](mode, methodName, undefined, requestDetails), ).to.eventually.equal(expected); }; it('should return true when the remaining budget is zero', async function () { - await testIsDailyBudgetExceeded(0, true); + await testIsTotalBudgetExceeded(0, true); }); it('should return true when the remaining budget is negative', async function () { - await testIsDailyBudgetExceeded(-1, true); + await testIsTotalBudgetExceeded(-1, true); }); it('should return false when the remaining budget is greater than zero', async function () { - await testIsDailyBudgetExceeded(100, false); + await testIsTotalBudgetExceeded(100, false); }); - it('should update the hbar limit counter when a method is called and the daily budget is exceeded', async function () { + it('should update the hbar limit counter when a method is called and the total budget is exceeded', async function () { // @ts-ignore const hbarLimitCounterSpy = sinon.spy(hbarLimitService.hbarLimitCounter, 'inc'); - await testIsDailyBudgetExceeded(0, true); + await testIsTotalBudgetExceeded(0, true); expect(hbarLimitCounterSpy.calledWithMatch({ mode, methodName }, 1)).to.be.true; }); @@ -742,7 +742,7 @@ describe('HbarLimitService', function () { // @ts-ignore hbarLimitService.reset = new Date(); const resetLimiterSpy = sinon.spy(hbarLimitService, 'resetLimiter'); - await testIsDailyBudgetExceeded(0, false); + await testIsTotalBudgetExceeded(0, false); expect(resetLimiterSpy.calledOnce).to.be.true; }); }); From 0da22ff3c973046c29ab6c5ef4e17eb821f6bd72 Mon Sep 17 00:00:00 2001 From: Victor Yanev Date: Fri, 11 Oct 2024 18:31:42 +0300 Subject: [PATCH 2/2] fix: jsdocs Signed-off-by: Victor Yanev --- packages/relay/src/lib/services/hbarLimitService/index.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/relay/src/lib/services/hbarLimitService/index.ts b/packages/relay/src/lib/services/hbarLimitService/index.ts index cffe64c7d4..0596abb3d6 100644 --- a/packages/relay/src/lib/services/hbarLimitService/index.ts +++ b/packages/relay/src/lib/services/hbarLimitService/index.ts @@ -50,7 +50,11 @@ export class HbarLimitService implements IHbarLimitService { private readonly hbarLimitRemainingGauge: Gauge; /** - * Tracks the number of unique ETH addresses that have made requests. + * Tracks the number of unique spending plans that have been utilized during the limit duration. + * (i.e., plans that had expenses added to them). + * + * For basic spending plans, this equates to the number of unique users who have made requests during that period, + * since each user has their own individual spending plan. * * @private */