Skip to content

Commit 7e87f4c

Browse files
entuziazSgtPookiCopilot
authored
fix: add shared guard to prevent zero pricing (#237)
* fix: add shared guard to prevent zero pricePerTiBPerEpoch in capacity and storage calculations - Introduced assertPriceNonZero() in core/utils/validate-pricing.ts - Integrated into calculateActualCapacity() and calculateStorageFromUSDFC() - Updated tests to expect throws on zero pricing - Prevents Infinity / unsafe deposit calculations * Update src/core/utils/validate-pricing.ts Co-authored-by: Copilot <[email protected]> * linting --------- Co-authored-by: Russell Dempsey <[email protected]> Co-authored-by: Copilot <[email protected]>
1 parent dda7d35 commit 7e87f4c

File tree

3 files changed

+38
-18
lines changed

3 files changed

+38
-18
lines changed

src/core/payments/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import { SIZE_CONSTANTS, type Synapse, TIME_CONSTANTS, TOKENS } from '@filoz/synapse-sdk'
1919
import { ethers } from 'ethers'
2020
import { isSessionKeyMode } from '../synapse/index.js'
21+
import { assertPriceNonZero } from '../utils/validate-pricing.js'
2122
import {
2223
BUFFER_DENOMINATOR,
2324
BUFFER_NUMERATOR,
@@ -508,7 +509,7 @@ export function calculateStorageAllowances(storageTiB: number, pricePerTiBPerEpo
508509
* @returns Storage capacity in TiB that can be supported
509510
*/
510511
export function calculateActualCapacity(rateAllowance: bigint, pricePerTiBPerEpoch: bigint): number {
511-
if (pricePerTiBPerEpoch === 0n) return 0
512+
assertPriceNonZero(pricePerTiBPerEpoch)
512513

513514
// Calculate TiB capacity from rate allowance
514515
const scaledQuotient = (rateAllowance * STORAGE_SCALE_MAX_BI) / pricePerTiBPerEpoch
@@ -536,7 +537,7 @@ export function calculateActualCapacity(rateAllowance: bigint, pricePerTiBPerEpo
536537
* @returns Storage capacity in TiB/month
537538
*/
538539
export function calculateStorageFromUSDFC(usdfcAmount: bigint, pricePerTiBPerEpoch: bigint): number {
539-
if (pricePerTiBPerEpoch === 0n) return 0
540+
assertPriceNonZero(pricePerTiBPerEpoch)
540541

541542
// Calculate how much this covers for lockup
542543
const epochsInLockupDays = BigInt(DEFAULT_LOCKUP_DAYS) * TIME_CONSTANTS.EPOCHS_PER_DAY

src/core/utils/validate-pricing.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* Validation utility for core financial calculations.
3+
* Keeps pricing inputs non-zero and positive.
4+
*
5+
* @param pricePerTiBPerEpoch - Current pricing from storage service
6+
*/
7+
8+
export function assertPriceNonZero(pricePerTiBPerEpoch: bigint): void {
9+
if (pricePerTiBPerEpoch <= 0n) {
10+
throw new Error('Invalid pricePerTiBPerEpoch: must be positive non-zero value')
11+
}
12+
}

src/test/unit/payments-setup.test.ts

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
setServiceApprovals,
1212
} from '../../core/payments/index.js'
1313
import { formatFIL, formatUSDFC } from '../../core/utils/format.js'
14+
import { assertPriceNonZero } from '../../core/utils/validate-pricing.js'
1415
import { parseStorageAllowance } from '../../payments/setup.js'
1516

1617
// Mock Synapse SDK
@@ -57,6 +58,20 @@ vi.mock('@filoz/synapse-sdk', () => {
5758
}
5859
})
5960

61+
describe('assertPriceNonZero', () => {
62+
it('throws when pricePerTiBPerEpoch is zero', () => {
63+
expect(() => assertPriceNonZero(0n)).toThrow('Invalid pricePerTiBPerEpoch: must be positive non-zero value')
64+
})
65+
66+
it('throws when pricePerTiBPerEpoch is negative', () => {
67+
expect(() => assertPriceNonZero(-10n)).toThrow('Invalid pricePerTiBPerEpoch: must be positive non-zero value')
68+
})
69+
70+
it('does not throw for positive price', () => {
71+
expect(() => assertPriceNonZero(1n)).not.toThrow()
72+
})
73+
})
74+
6075
describe('Payment Setup Tests', () => {
6176
let mockSynapse: any
6277
let mockProvider: any
@@ -342,9 +357,10 @@ describe('Payment Setup Tests', () => {
342357
expect(capacityTiB).toBeCloseTo(expectedTiB, 5)
343358
})
344359

345-
it('should handle zero price gracefully', () => {
346-
const capacityTiB = calculateActualCapacity(ethers.parseUnits('1', 18), 0n)
347-
expect(capacityTiB).toBe(0)
360+
it('throws when pricePerTiBPerEpoch is zero', () => {
361+
expect(() => calculateActualCapacity(ethers.parseUnits('1', 18), 0n)).toThrow(
362+
'Invalid pricePerTiBPerEpoch: must be positive non-zero value'
363+
)
348364
})
349365
})
350366

@@ -360,19 +376,10 @@ describe('Payment Setup Tests', () => {
360376
expect(capacityTiB).toBeCloseTo(expectedTiB, 5)
361377
})
362378

363-
it('should handle zero price gracefully', () => {
364-
const capacityTiB = calculateStorageFromUSDFC(ethers.parseUnits('1', 18), 0n)
365-
expect(capacityTiB).toBe(0)
366-
})
367-
368-
// FIXME: if pricePerTiBPerEpoch is 0, shouldn't we throw an error or return Infinity?
369-
// See https://github.com/filecoin-project/filecoin-pin/issues/38
370-
it('returns 0 if pricePerTiBPerEpoch is 0', () => {
371-
const usdfcAmount = ethers.parseUnits('1', 18)
372-
const pricePerTiBPerEpoch = ethers.parseUnits('0', 18)
373-
const capacityTiB = calculateStorageFromUSDFC(usdfcAmount, pricePerTiBPerEpoch)
374-
375-
expect(capacityTiB).toBe(0)
379+
it('throws when pricePerTiBPerEpoch is zero', () => {
380+
expect(() => calculateStorageFromUSDFC(ethers.parseUnits('1', 18), 0n)).toThrow(
381+
'Invalid pricePerTiBPerEpoch: must be positive non-zero value'
382+
)
376383
})
377384

378385
it('returns 0 if usdfcAmount is 0', () => {

0 commit comments

Comments
 (0)