From df673f94c0a6f92d01b565ac079c1df3234fd896 Mon Sep 17 00:00:00 2001 From: Daniel Padrino Date: Mon, 13 Jul 2026 14:05:35 -0300 Subject: [PATCH] fix(pricing): validate asset prices before storing and guard the DEX reference amount --- .../asset/__tests__/asset.entity.spec.ts | 47 ++++++++ src/shared/models/asset/asset.entity.ts | 10 +- .../asset-prices-job.service.spec.ts | 101 ++++++++++++++++++ .../services/asset-prices-job.service.ts | 11 +- 4 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 src/shared/models/asset/__tests__/asset.entity.spec.ts create mode 100644 src/subdomains/supporting/pricing/services/__tests__/asset-prices-job.service.spec.ts diff --git a/src/shared/models/asset/__tests__/asset.entity.spec.ts b/src/shared/models/asset/__tests__/asset.entity.spec.ts new file mode 100644 index 0000000000..ccf61eb387 --- /dev/null +++ b/src/shared/models/asset/__tests__/asset.entity.spec.ts @@ -0,0 +1,47 @@ +import { createCustomAsset } from '../__mocks__/asset.entity.mock'; +import { Asset } from '../asset.entity'; + +describe('Asset', () => { + describe('#isSanePrice', () => { + it('accepts regular prices', () => { + expect(Asset.isSanePrice(0.0061)).toBe(true); + expect(Asset.isSanePrice(1)).toBe(true); + expect(Asset.isSanePrice(100000)).toBe(true); + }); + + it('rejects unset, zero, non-finite and degenerate prices', () => { + expect(Asset.isSanePrice(undefined)).toBe(false); + expect(Asset.isSanePrice(0)).toBe(false); + expect(Asset.isSanePrice(NaN)).toBe(false); + expect(Asset.isSanePrice(Infinity)).toBe(false); + expect(Asset.isSanePrice(4.2421310463457016e-60)).toBe(false); + expect(Asset.isSanePrice(1e60)).toBe(false); + }); + }); + + describe('#minimalPriceReferenceAmount', () => { + it('returns the inverse of a sane approxPriceChf', () => { + const asset = createCustomAsset({ approxPriceChf: 0.5 }); + + expect(asset.minimalPriceReferenceAmount).toBe(2); + }); + + it('falls back to 1 when approxPriceChf is unset', () => { + const asset = createCustomAsset({ approxPriceChf: undefined }); + + expect(asset.minimalPriceReferenceAmount).toBe(1); + }); + + it('falls back to 1 when approxPriceChf is 0', () => { + const asset = createCustomAsset({ approxPriceChf: 0 }); + + expect(asset.minimalPriceReferenceAmount).toBe(1); + }); + + it('falls back to 1 for a degenerate near-zero price instead of inverting it into an overflow', () => { + const asset = createCustomAsset({ approxPriceChf: 4.2421310463457016e-60 }); + + expect(asset.minimalPriceReferenceAmount).toBe(1); + }); + }); +}); diff --git a/src/shared/models/asset/asset.entity.ts b/src/shared/models/asset/asset.entity.ts index f1520becaa..28b2f15832 100644 --- a/src/shared/models/asset/asset.entity.ts +++ b/src/shared/models/asset/asset.entity.ts @@ -128,8 +128,16 @@ export class Asset extends IEntity { @ManyToOne(() => PriceRule) priceRule: PriceRule; + // prices outside these rails are corrupted data (e.g. from a degenerate DEX quote), not real prices + static readonly MIN_SANE_PRICE = 1e-12; + static readonly MAX_SANE_PRICE = 1e15; + + static isSanePrice(price?: number): boolean { + return price != null && Number.isFinite(price) && price > Asset.MIN_SANE_PRICE && price < Asset.MAX_SANE_PRICE; + } + get minimalPriceReferenceAmount() { - return this.approxPriceChf ? 1 / this.approxPriceChf : 1; + return Asset.isSanePrice(this.approxPriceChf) ? 1 / this.approxPriceChf : 1; } get evmChainId(): number | undefined { diff --git a/src/subdomains/supporting/pricing/services/__tests__/asset-prices-job.service.spec.ts b/src/subdomains/supporting/pricing/services/__tests__/asset-prices-job.service.spec.ts new file mode 100644 index 0000000000..b71b9a6e0e --- /dev/null +++ b/src/subdomains/supporting/pricing/services/__tests__/asset-prices-job.service.spec.ts @@ -0,0 +1,101 @@ +import { createMock } from '@golevelup/ts-jest'; +import { Test, TestingModule } from '@nestjs/testing'; +import { createCustomAsset } from 'src/shared/models/asset/__mocks__/asset.entity.mock'; +import { AssetType } from 'src/shared/models/asset/asset.entity'; +import { AssetService } from 'src/shared/models/asset/asset.service'; +import { FiatService } from 'src/shared/models/fiat/fiat.service'; +import { DfxLogger } from 'src/shared/services/dfx-logger'; +import { TestUtil } from 'src/shared/utils/test.util'; +import { AssetPriceRepository } from '../../repositories/asset-price.repository'; +import { AssetPricesJobService } from '../asset-prices-job.service'; +import { PricingService } from '../pricing.service'; + +describe('AssetPricesJobService', () => { + let service: AssetPricesJobService; + + let assetService: AssetService; + let fiatService: FiatService; + let pricingService: PricingService; + let assetPriceRepo: AssetPriceRepository; + + beforeEach(async () => { + assetService = createMock(); + fiatService = createMock(); + pricingService = createMock(); + assetPriceRepo = createMock(); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + AssetPricesJobService, + { provide: AssetService, useValue: assetService }, + { provide: FiatService, useValue: fiatService }, + { provide: PricingService, useValue: pricingService }, + { provide: AssetPriceRepository, useValue: assetPriceRepo }, + TestUtil.provideConfig(), + ], + }).compile(); + + service = module.get(AssetPricesJobService); + }); + + afterEach(() => jest.restoreAllMocks()); + + const priceOf = (value: number) => ({ convert: () => value }) as any; + + describe('#updatePrices', () => { + it('stores sane prices', async () => { + const asset = createCustomAsset({ id: 264, uniqueName: 'Arbitrum/TGT', type: AssetType.CUSTOM }); + jest.spyOn(assetService, 'getPricedAssets').mockResolvedValue([asset]); + jest + .spyOn(pricingService, 'getPrice') + .mockResolvedValueOnce(priceOf(0.0075)) + .mockResolvedValueOnce(priceOf(0.0061)) + .mockResolvedValueOnce(priceOf(0.0064)); + + await service.updatePrices(); + + expect(assetService.updatePrices).toHaveBeenCalledWith([ + [264, { approxPriceUsd: 0.0075, approxPriceChf: 0.0061, approxPriceEur: 0.0064 }], + ]); + }); + + it('skips the update and warns when a fetched price is degenerate', async () => { + const warnSpy = jest.spyOn(DfxLogger.prototype, 'warn').mockImplementation(() => undefined); + const asset = createCustomAsset({ + id: 264, + uniqueName: 'Arbitrum/TGT', + type: AssetType.CUSTOM, + approxPriceChf: 0.0061, + }); + jest.spyOn(assetService, 'getPricedAssets').mockResolvedValue([asset]); + jest.spyOn(pricingService, 'getPrice').mockResolvedValue(priceOf(4.2421310463457016e-60)); + + await service.updatePrices(); + + expect(assetService.updatePrices).toHaveBeenCalledWith([]); + expect(asset.approxPriceChf).toBe(0.0061); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Arbitrum/TGT')); + }); + + it('still updates the remaining assets when one price is degenerate', async () => { + jest.spyOn(DfxLogger.prototype, 'warn').mockImplementation(() => undefined); + const degenerate = createCustomAsset({ id: 264, uniqueName: 'Arbitrum/TGT', type: AssetType.CUSTOM }); + const sane = createCustomAsset({ id: 265, uniqueName: 'Arbitrum/USDT', type: AssetType.CUSTOM }); + jest.spyOn(assetService, 'getPricedAssets').mockResolvedValue([degenerate, sane]); + jest + .spyOn(pricingService, 'getPrice') + .mockResolvedValueOnce(priceOf(4.2421310463457016e-60)) + .mockResolvedValueOnce(priceOf(4.2421310463457016e-60)) + .mockResolvedValueOnce(priceOf(4.2421310463457016e-60)) + .mockResolvedValueOnce(priceOf(1.07)) + .mockResolvedValueOnce(priceOf(0.86)) + .mockResolvedValueOnce(priceOf(0.92)); + + await service.updatePrices(); + + expect(assetService.updatePrices).toHaveBeenCalledWith([ + [265, { approxPriceUsd: 1.07, approxPriceChf: 0.86, approxPriceEur: 0.92 }], + ]); + }); + }); +}); diff --git a/src/subdomains/supporting/pricing/services/asset-prices-job.service.ts b/src/subdomains/supporting/pricing/services/asset-prices-job.service.ts index 1bb7257838..addfbfe69a 100644 --- a/src/subdomains/supporting/pricing/services/asset-prices-job.service.ts +++ b/src/subdomains/supporting/pricing/services/asset-prices-job.service.ts @@ -36,7 +36,16 @@ export class AssetPricesJobService { const chfPrice = await this.pricingService.getPrice(asset, PriceCurrency.CHF, PriceValidity.VALID_ONLY); const eurPrice = await this.pricingService.getPrice(asset, PriceCurrency.EUR, PriceValidity.VALID_ONLY); - updates.push(asset.updatePrice(usdPrice.convert(1), chfPrice.convert(1), eurPrice.convert(1))); + const [usd, chf, eur] = [usdPrice, chfPrice, eurPrice].map((p) => p.convert(1)); + + if (![usd, chf, eur].every((p) => Asset.isSanePrice(p))) { + this.logger.warn( + `Skipping price update of asset ${asset.uniqueName}: degenerate price (USD ${usd}, CHF ${chf}, EUR ${eur})`, + ); + continue; + } + + updates.push(asset.updatePrice(usd, chf, eur)); if ([AssetType.COIN, AssetType.TOKEN, AssetType.CUSTODY].includes(asset.type)) await this.saveAssetPrices(asset);