diff --git a/packages/bitcore-wallet-service/src/externalservices/coingecko.ts b/packages/bitcore-wallet-service/src/externalservices/coingecko.ts index f6744cb679..44f866569f 100644 --- a/packages/bitcore-wallet-service/src/externalservices/coingecko.ts +++ b/packages/bitcore-wallet-service/src/externalservices/coingecko.ts @@ -36,8 +36,13 @@ type CoinGeckoRequest = Pick; const SUPPORTED_FIAT_CODES = new Set(Defaults.FIAT_CURRENCIES.map(f => f.code)); -const ALLOWED_DAYS = new Set([1, 7, 30, 90, 365, 1825]); -const DEFAULT_ALL_TIME_DAYS = 100000; +const ALLOWED_NUMERIC_DAYS = new Set([1, 7, 30, 90, 365, 1825]); + +// CoinGecko's documented all-time value for `market_chart` is the literal string `max` +const ALL_TIME_DAYS = 'max'; + +type FiatRateNumericDays = 1 | 7 | 30 | 90 | 365 | 1825; +type FiatRateDays = FiatRateNumericDays | typeof ALL_TIME_DAYS; const MAX_QUERY_PARAM_LENGTH = 64; const CHAIN_PARAM_PATTERN = /^[a-z0-9-_]+$/i; @@ -122,6 +127,16 @@ function validateQueryParam(query: Record | undefined, key: str } } +function isAllowedNumericDays(value: number): value is FiatRateNumericDays { + return ALLOWED_NUMERIC_DAYS.has(value); +} + +function parseFiatRateNumericDays(raw: string): FiatRateNumericDays { + const parsed = +raw; + if (!isAllowedNumericDays(parsed)) throw badRequest('Invalid days'); + return parsed; +} + function parseCoinChainTokenAddress( query: Record | undefined ): { coin?: string; chain?: string; tokenAddress?: string } { @@ -320,12 +335,12 @@ export class CoinGeckoService { throw badRequest('Unsupported coin. For token symbols, pass `chain` and `tokenAddress`.'); } - private async fetchFiatRatesForId(id: string, vsCurrency: string, days: number): Promise { + private async fetchFiatRatesForId(id: string, vsCurrency: string, days: FiatRateDays): Promise { const { API_KEY } = this.coinGeckoGetCredentials(); const headers = this.coinGeckoGetHeaders(API_KEY); - const params: any = { vs_currency: vsCurrency, days }; - if (days >= 90) params.interval = 'daily'; + const params: Record = { vs_currency: vsCurrency, days }; + if (days === ALL_TIME_DAYS || days >= 90) params.interval = 'daily'; const url = this.cgUrl(`/v3/coins/${encodeURIComponent(id)}/market_chart`, params); const body = await this.coinGeckoGetJson(url, headers); @@ -341,7 +356,7 @@ export class CoinGeckoService { private async getFiatRatesForId( id: string, vsCurrency: string, - days: number, + days: FiatRateDays, useDbCache: boolean ): Promise { if (!useDbCache) return this.fetchFiatRatesForId(id, vsCurrency, days); @@ -465,13 +480,12 @@ export class CoinGeckoService { const { coin: coinParam, chain: chainParam, tokenAddress: tokenAddressParam } = parseCoinChainTokenAddress(req.query); - let days = DEFAULT_ALL_TIME_DAYS; + // An omitted `days` means ALL-time history. + let days: FiatRateDays = ALL_TIME_DAYS; try { const daysParam = getQueryString(req.query, 'days'); if (daysParam !== undefined) { - const parsed = +daysParam; - if (!ALLOWED_DAYS.has(parsed)) throw badRequest('Invalid days'); - days = parsed; + days = parseFiatRateNumericDays(daysParam); } } catch { throw badRequest('Invalid days'); diff --git a/packages/bitcore-wallet-service/test/integration/externalservices/coinGecko.test.ts b/packages/bitcore-wallet-service/test/integration/externalservices/coinGecko.test.ts index b15c56b5d0..bf6581f7c8 100644 --- a/packages/bitcore-wallet-service/test/integration/externalservices/coinGecko.test.ts +++ b/packages/bitcore-wallet-service/test/integration/externalservices/coinGecko.test.ts @@ -150,6 +150,22 @@ describe('CoinGecko integration', function() { sandbox.stub(cg().storage, 'storeGlobalCache').callsFake((_key, _values, cb) => cb(null)); } + /** Wraps the active fakeRequest so every outgoing market_chart URL is recorded, in order. */ + function captureMarketChartUrls(): string[] { + const urls: string[] = []; + const baseGet = fakeRequest.get; + fakeRequest.get = (url, opts, cb) => { + if (matchCoinIdFromMarketChartUrl(url)) urls.push(url); + return baseGet(url, opts, cb); + }; + return urls; + } + + function daysAndInterval(url: string): { days: string | null; interval: string | null } { + const u = toURL(url); + return { days: u.searchParams.get('days'), interval: u.searchParams.get('interval') }; + } + before(async () => { await helpers.before(); }); @@ -840,9 +856,9 @@ describe('CoinGecko integration', function() { should.exist(data); checkCacheStub.callCount.should.equal(1); - checkCacheStub.getCall(0).args[0].should.equal('cgFiatRates:bitcoin:usd:100000'); + checkCacheStub.getCall(0).args[0].should.equal('cgFiatRates:bitcoin:usd:max'); storeCacheStub.callCount.should.equal(1); - storeCacheStub.getCall(0).args[0].should.equal('cgFiatRates:bitcoin:usd:100000'); + storeCacheStub.getCall(0).args[0].should.equal('cgFiatRates:bitcoin:usd:max'); }); it('should bypass DB cache for token fiatrates requests', async () => { @@ -881,12 +897,20 @@ describe('CoinGecko integration', function() { }); it('should return error on invalid days', async () => { - try { - await getFiatRates({ coin: 'BTC', days: 0 }); - should.fail('should have thrown'); - } catch (err) { - err.message.should.equal('Invalid days'); + const getSpy = sandbox.spy(); + cg().request = { get: getSpy }; + + for (const days of [0, 100000, 'max', 'MAX']) { + try { + await getFiatRates({ coin: 'BTC', days }); + should.fail(`should have thrown for days=${JSON.stringify(days)}`); + } catch (err) { + err.message.should.equal('Invalid days'); + err.statusCode.should.equal(400); + } } + + getSpy.callCount.should.equal(0); }); it('should return rates for default coins if missing coin', async () => { @@ -935,5 +959,45 @@ describe('CoinGecko integration', function() { .filter(args => args?.[0]?.toString().includes('CoinGecko fiat rates fetch failed') && args?.[1]?.coin === 'bch'); bchWarns.length.should.be.greaterThan(0); }); + + describe('all-time window', () => { + it('should request days=max when days is omitted', async () => { + forceGlobalCacheMisses(); + const urls = captureMarketChartUrls(); + + const data = await getFiatRates({ coin: 'BTC' }); + + data.should.deep.equal([ + { ts: 1, rate: 100 }, + { ts: 2, rate: 110 } + ]); + urls.should.have.length(1); + daysAndInterval(urls[0]).should.deep.equal({ days: 'max', interval: 'daily' }); + }); + }); + + describe('numeric day windows', () => { + it('should keep every allowed numeric window accepted and unchanged', async () => { + forceGlobalCacheMisses(); + const urls = captureMarketChartUrls(); + + for (const days of [1, 7, 30, 90, 365, 1825]) { + const data = await getFiatRates({ coin: 'BTC', days }); + data.should.deep.equal([ + { ts: 1, rate: 100 }, + { ts: 2, rate: 110 } + ]); + } + + urls.map(daysAndInterval).should.deep.equal([ + { days: '1', interval: null }, + { days: '7', interval: null }, + { days: '30', interval: null }, + { days: '90', interval: 'daily' }, + { days: '365', interval: 'daily' }, + { days: '1825', interval: 'daily' } + ]); + }); + }); }); });