diff --git a/packages/assets-controllers/package.json b/packages/assets-controllers/package.json index e00d315c6ae..d11ce9d57a2 100644 --- a/packages/assets-controllers/package.json +++ b/packages/assets-controllers/package.json @@ -48,6 +48,7 @@ "@metamask/utils": "^8.2.0", "@types/uuid": "^8.3.0", "async-mutex": "^0.2.6", + "cockatiel": "3.1.1", "ethereumjs-util": "^7.0.10", "lodash": "^4.17.21", "multiformats": "^9.5.2", diff --git a/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts b/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts index 8819504f2c9..5f41bdda49a 100644 --- a/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts +++ b/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts @@ -1,12 +1,15 @@ import nock from 'nock'; +import { useFakeTimers } from 'sinon'; import { - codefiTokenPricesServiceV2, + CodefiTokenPricesServiceV2, SUPPORTED_CHAIN_IDS, SUPPORTED_CURRENCIES, } from './codefi-v2'; -describe('codefiTokenPricesServiceV2', () => { +const defaultMaxRetryDelay = 30_000; + +describe('new CodefiTokenPricesServiceV2()', () => { describe('fetchTokenPrices', () => { it('uses the /spot-prices endpoint of the Codefi Price API to gather prices for the given tokens', async () => { nock('https://price-api.metafi.codefi.network') @@ -28,7 +31,7 @@ describe('codefiTokenPricesServiceV2', () => { }); const pricedTokensByAddress = - await codefiTokenPricesServiceV2.fetchTokenPrices({ + await new CodefiTokenPricesServiceV2().fetchTokenPrices({ chainId: '0x1', tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], currency: 'ETH', @@ -70,7 +73,7 @@ describe('codefiTokenPricesServiceV2', () => { }); await expect( - codefiTokenPricesServiceV2.fetchTokenPrices({ + new CodefiTokenPricesServiceV2().fetchTokenPrices({ chainId: '0x1', tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], currency: 'ETH', @@ -96,13 +99,385 @@ describe('codefiTokenPricesServiceV2', () => { }); await expect( - codefiTokenPricesServiceV2.fetchTokenPrices({ + new CodefiTokenPricesServiceV2().fetchTokenPrices({ chainId: '0x1', tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], currency: 'ETH', }), ).rejects.toThrow('Could not find price for "0xAAA" in "ETH"'); }); + + it('throws if the request fails consistently', async () => { + nock('https://price-api.metafi.codefi.network') + .get('/v2/chains/1/spot-prices') + .query({ + tokenAddresses: '0xAAA,0xBBB,0xCCC', + vsCurrency: 'ETH', + }) + .replyWithError('Failed to fetch') + .persist(); + + await expect( + new CodefiTokenPricesServiceV2().fetchTokenPrices({ + chainId: '0x1', + tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], + currency: 'ETH', + }), + ).rejects.toThrow('Failed to fetch'); + }); + + it('throws if the initial request and all retries fail', async () => { + const retries = 3; + nock('https://price-api.metafi.codefi.network') + .get('/v2/chains/1/spot-prices') + .query({ + tokenAddresses: '0xAAA,0xBBB,0xCCC', + vsCurrency: 'ETH', + }) + .times(1 + retries) + .replyWithError('Failed to fetch'); + + await expect( + new CodefiTokenPricesServiceV2({ retries }).fetchTokenPrices({ + chainId: '0x1', + tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], + currency: 'ETH', + }), + ).rejects.toThrow('Failed to fetch'); + }); + + it('succeeds if the last retry succeeds', async () => { + const retries = 3; + // Initial interceptor for failing requests + nock('https://price-api.metafi.codefi.network') + .get('/v2/chains/1/spot-prices') + .query({ + tokenAddresses: '0xAAA,0xBBB,0xCCC', + vsCurrency: 'ETH', + }) + .times(retries) + .replyWithError('Failed to fetch'); + // Interceptor for successful request + nock('https://price-api.metafi.codefi.network') + .get('/v2/chains/1/spot-prices') + .query({ + tokenAddresses: '0xAAA,0xBBB,0xCCC', + vsCurrency: 'ETH', + }) + .reply(200, { + '0xaaa': { + eth: 148.17205755299946, + }, + '0xbbb': { + eth: 33689.98134554716, + }, + '0xccc': { + eth: 148.1344197578456, + }, + }); + + const pricedTokensByAddress = await new CodefiTokenPricesServiceV2({ + retries, + }).fetchTokenPrices({ + chainId: '0x1', + tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], + currency: 'ETH', + }); + + expect(pricedTokensByAddress).toStrictEqual({ + '0xAAA': { + tokenAddress: '0xAAA', + value: 148.17205755299946, + currency: 'ETH', + }, + '0xBBB': { + tokenAddress: '0xBBB', + value: 33689.98134554716, + currency: 'ETH', + }, + '0xCCC': { + tokenAddress: '0xCCC', + value: 148.1344197578456, + currency: 'ETH', + }, + }); + }); + + describe('after circuit break', () => { + let clock: sinon.SinonFakeTimers; + + beforeEach(() => { + clock = useFakeTimers({ now: Date.now() }); + }); + + afterEach(() => { + clock.restore(); + }); + + it('stops making fetch requests after too many consecutive failures', async () => { + const retries = 3; + // Max consencutive failures is set to match number of calls in three update attempts (including retries) + const maximumConsecutiveFailures = (1 + retries) * 3; + // Initial interceptor for failing requests + nock('https://price-api.metafi.codefi.network') + .get('/v2/chains/1/spot-prices') + .query({ + tokenAddresses: '0xAAA,0xBBB,0xCCC', + vsCurrency: 'ETH', + }) + .times(maximumConsecutiveFailures) + .replyWithError('Failed to fetch'); + // This interceptor should not be used + const successfullCallScope = nock( + 'https://price-api.metafi.codefi.network', + ) + .get('/v2/chains/1/spot-prices') + .query({ + tokenAddresses: '0xAAA,0xBBB,0xCCC', + vsCurrency: 'ETH', + }) + .reply(200, { + '0xaaa': { + eth: 148.17205755299946, + }, + '0xbbb': { + eth: 33689.98134554716, + }, + '0xccc': { + eth: 148.1344197578456, + }, + }); + const service = new CodefiTokenPricesServiceV2({ + retries, + maximumConsecutiveFailures, + // Ensure break duration is well over the max delay for a single request, so that the + // break doesn't end during a retry attempt + circuitBreakDuration: defaultMaxRetryDelay * 10, + }); + const fetchTokenPrices = () => + service.fetchTokenPrices({ + chainId: '0x1', + tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], + currency: 'ETH', + }); + // Initial three calls to exhaust maximum allowed failures + // eslint-disable-next-line @typescript-eslint/no-unused-vars + for (const _retryAttempt of Array(retries).keys()) { + // eslint-disable-next-line no-loop-func + await expect(() => + fetchTokenPricesWithFakeTimers({ + clock, + fetchTokenPrices, + retries, + }), + ).rejects.toThrow('Failed to fetch'); + } + + await expect(() => + fetchTokenPricesWithFakeTimers({ + clock, + fetchTokenPrices, + retries, + }), + ).rejects.toThrow( + 'Execution prevented because the circuit breaker is open', + ); + expect(successfullCallScope.isDone()).toBe(false); + }); + + it('keeps circuit closed if first request fails when half-open', async () => { + const retries = 3; + // Max consencutive failures is set to match number of calls in three update attempts (including retries) + const maximumConsecutiveFailures = (1 + retries) * 3; + // Ensure break duration is well over the max delay for a single request, so that the + // break doesn't end during a retry attempt + const circuitBreakDuration = defaultMaxRetryDelay * 10; + // Initial interceptor for failing requests + nock('https://price-api.metafi.codefi.network') + .get('/v2/chains/1/spot-prices') + .query({ + tokenAddresses: '0xAAA,0xBBB,0xCCC', + vsCurrency: 'ETH', + }) + // The +1 is for the additional request when the circuit is half-open + .times(maximumConsecutiveFailures + 1) + .replyWithError('Failed to fetch'); + // This interceptor should not be used + const successfullCallScope = nock( + 'https://price-api.metafi.codefi.network', + ) + .get('/v2/chains/1/spot-prices') + .query({ + tokenAddresses: '0xAAA,0xBBB,0xCCC', + vsCurrency: 'ETH', + }) + .reply(200, { + '0xaaa': { + eth: 148.17205755299946, + }, + '0xbbb': { + eth: 33689.98134554716, + }, + '0xccc': { + eth: 148.1344197578456, + }, + }); + const service = new CodefiTokenPricesServiceV2({ + retries, + maximumConsecutiveFailures, + circuitBreakDuration, + }); + const fetchTokenPrices = () => + service.fetchTokenPrices({ + chainId: '0x1', + tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], + currency: 'ETH', + }); + // Initial three calls to exhaust maximum allowed failures + // eslint-disable-next-line @typescript-eslint/no-unused-vars + for (const _retryAttempt of Array(retries).keys()) { + // eslint-disable-next-line no-loop-func + await expect(() => + fetchTokenPricesWithFakeTimers({ + clock, + fetchTokenPrices, + retries, + }), + ).rejects.toThrow('Failed to fetch'); + } + // Confirm that circuit has broken + await expect(() => + fetchTokenPricesWithFakeTimers({ + clock, + fetchTokenPrices, + retries, + }), + ).rejects.toThrow( + 'Execution prevented because the circuit breaker is open', + ); + // Wait for circuit to move to half-open + await clock.tickAsync(circuitBreakDuration); + + // The circuit should remain open after the first request fails + // The fetch error is replaced by the circuit break error due to the retries + await expect(() => + fetchTokenPricesWithFakeTimers({ + clock, + fetchTokenPrices, + retries, + }), + ).rejects.toThrow( + 'Execution prevented because the circuit breaker is open', + ); + + // Confirm that the circuit is still open + await expect(() => + fetchTokenPricesWithFakeTimers({ + clock, + fetchTokenPrices, + retries, + }), + ).rejects.toThrow( + 'Execution prevented because the circuit breaker is open', + ); + expect(successfullCallScope.isDone()).toBe(false); + }); + + it('recovers after circuit break', async () => { + const retries = 3; + // Max consencutive failures is set to match number of calls in three update attempts (including retries) + const maximumConsecutiveFailures = (1 + retries) * 3; + // Ensure break duration is well over the max delay for a single request, so that the + // break doesn't end during a retry attempt + const circuitBreakDuration = defaultMaxRetryDelay * 10; + // Initial interceptor for failing requests + nock('https://price-api.metafi.codefi.network') + .get('/v2/chains/1/spot-prices') + .query({ + tokenAddresses: '0xAAA,0xBBB,0xCCC', + vsCurrency: 'ETH', + }) + .times(maximumConsecutiveFailures) + .replyWithError('Failed to fetch'); + // Later interceptor for successfull request after recovery + nock('https://price-api.metafi.codefi.network') + .get('/v2/chains/1/spot-prices') + .query({ + tokenAddresses: '0xAAA,0xBBB,0xCCC', + vsCurrency: 'ETH', + }) + .reply(200, { + '0xaaa': { + eth: 148.17205755299946, + }, + '0xbbb': { + eth: 33689.98134554716, + }, + '0xccc': { + eth: 148.1344197578456, + }, + }); + const service = new CodefiTokenPricesServiceV2({ + retries, + maximumConsecutiveFailures, + circuitBreakDuration, + }); + const fetchTokenPrices = () => + service.fetchTokenPrices({ + chainId: '0x1', + tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], + currency: 'ETH', + }); + // Initial three calls to exhaust maximum allowed failures + // eslint-disable-next-line @typescript-eslint/no-unused-vars + for (const _retryAttempt of Array(retries).keys()) { + // eslint-disable-next-line no-loop-func + await expect(() => + fetchTokenPricesWithFakeTimers({ + clock, + fetchTokenPrices, + retries, + }), + ).rejects.toThrow('Failed to fetch'); + } + // Confirm that circuit has broken + await expect(() => + fetchTokenPricesWithFakeTimers({ + clock, + fetchTokenPrices, + retries, + }), + ).rejects.toThrow( + 'Execution prevented because the circuit breaker is open', + ); + // Wait for circuit to move to half-open + await clock.tickAsync(circuitBreakDuration); + + const pricedTokensByAddress = await fetchTokenPricesWithFakeTimers({ + clock, + fetchTokenPrices, + retries, + }); + + expect(pricedTokensByAddress).toStrictEqual({ + '0xAAA': { + tokenAddress: '0xAAA', + value: 148.17205755299946, + currency: 'ETH', + }, + '0xBBB': { + tokenAddress: '0xBBB', + value: 33689.98134554716, + currency: 'ETH', + }, + '0xCCC': { + tokenAddress: '0xCCC', + value: 148.1344197578456, + currency: 'ETH', + }, + }); + }); + }); }); describe('validateChainIdSupported', () => { @@ -110,14 +485,14 @@ describe('codefiTokenPricesServiceV2', () => { 'returns true if the given chain ID is %s', (chainId) => { expect( - codefiTokenPricesServiceV2.validateChainIdSupported(chainId), + new CodefiTokenPricesServiceV2().validateChainIdSupported(chainId), ).toBe(true); }, ); it('returns false if the given chain ID is not one of the supported chain IDs', () => { expect( - codefiTokenPricesServiceV2.validateChainIdSupported( + new CodefiTokenPricesServiceV2().validateChainIdSupported( '0x999999999999999', ), ).toBe(false); @@ -129,7 +504,7 @@ describe('codefiTokenPricesServiceV2', () => { 'returns true if the given currency is %s', (currency) => { expect( - codefiTokenPricesServiceV2.validateCurrencySupported(currency), + new CodefiTokenPricesServiceV2().validateCurrencySupported(currency), ).toBe(true); }, ); @@ -138,15 +513,54 @@ describe('codefiTokenPricesServiceV2', () => { 'returns true if the given currency is %s', (currency) => { expect( - codefiTokenPricesServiceV2.validateCurrencySupported(currency), + new CodefiTokenPricesServiceV2().validateCurrencySupported(currency), ).toBe(true); }, ); it('returns false if the given currency is not one of the supported currencies', () => { - expect(codefiTokenPricesServiceV2.validateCurrencySupported('LOL')).toBe( - false, - ); + expect( + new CodefiTokenPricesServiceV2().validateCurrencySupported('LOL'), + ).toBe(false); }); }); }); + +/** + * Calls the 'fetchTokenPrices' function while advancing the clock, allowing + * the function to resolve. + * + * Fetching token rates is challenging in an environment with fake timers + * because we're using a library that automatically retries failed requests, + * which uses `setTimeout` internally. We have to advance the clock after the + * update call starts but before awaiting the result, otherwise it never + * resolves. + * + * @param args - Arguments + * @param args.clock - The fake timers clock to advance. + * @param args.fetchTokenPrices - The "fetchTokenPrices" function to call. + * @param args.retries - The number of retries the fetch call is configured to make. + */ +async function fetchTokenPricesWithFakeTimers({ + clock, + fetchTokenPrices, + retries, +}: { + clock: sinon.SinonFakeTimers; + fetchTokenPrices: () => Promise; + retries: number; +}) { + const pendingUpdate = fetchTokenPrices(); + pendingUpdate.catch(() => { + // suppress Unhandled Promise error + }); + + // Advance timer enough to exceed max possible retry delay for initial call, and all + // subsequent retries + // eslint-disable-next-line @typescript-eslint/no-unused-vars + for (const _retryAttempt of Array(retries + 1).keys()) { + await clock.tickAsync(defaultMaxRetryDelay); + } + + return await pendingUpdate; +} diff --git a/packages/assets-controllers/src/token-prices-service/codefi-v2.ts b/packages/assets-controllers/src/token-prices-service/codefi-v2.ts index f1407ba2234..477122a9e8b 100644 --- a/packages/assets-controllers/src/token-prices-service/codefi-v2.ts +++ b/packages/assets-controllers/src/token-prices-service/codefi-v2.ts @@ -1,6 +1,15 @@ import { handleFetch } from '@metamask/controller-utils'; import type { Hex } from '@metamask/utils'; import { hexToNumber } from '@metamask/utils'; +import { + circuitBreaker, + ConsecutiveBreaker, + ExponentialBackoff, + handleAll, + type IPolicy, + retry, + wrap, +} from 'cockatiel'; import type { AbstractTokenPricesService, @@ -238,15 +247,53 @@ type SupportedChainId = (typeof SUPPORTED_CHAIN_IDS)[number]; */ const BASE_URL = 'https://price-api.metafi.codefi.network/v2'; +const DEFAULT_TOKEN_PRICE_RETRIES = 3; +// Each update attempt will result (1 + retries) calls if the server is down +const DEFAULT_TOKEN_PRICE_MAX_CONSECUTIVE_FAILURES = + (1 + DEFAULT_TOKEN_PRICE_RETRIES) * 3; + /** * This version of the token prices service uses V2 of the Codefi Price API to * fetch token prices. */ -export const codefiTokenPricesServiceV2: AbstractTokenPricesService< - SupportedChainId, - Hex, - SupportedCurrency -> = { +export class CodefiTokenPricesServiceV2 + implements + AbstractTokenPricesService +{ + #tokenPricePolicy: IPolicy; + + /** + * Construct a Codefi Token Price Service. + * + * @param options - Constructor options + * @param options.retries - Number of retry attempts for each token price update. + * @param options.maximumConsecutiveFailures - The maximum number of consecutive failures + * allowed before breaking the circuit and pausing further updates. + * @param options.circuitBreakDuration - The amount of time to wait when the circuit breaks + * from too many consecutive failures. + */ + constructor({ + retries = DEFAULT_TOKEN_PRICE_RETRIES, + maximumConsecutiveFailures = DEFAULT_TOKEN_PRICE_MAX_CONSECUTIVE_FAILURES, + circuitBreakDuration = 30 * 60 * 1000, + }: { + retries?: number; + maximumConsecutiveFailures?: number; + circuitBreakDuration?: number; + } = {}) { + // Construct a policy that will retry each update, and halt further updates + // for a certain period after too many consecutive failures. + const retryPolicy = retry(handleAll, { + maxAttempts: retries, + backoff: new ExponentialBackoff(), + }); + const circuitBreakerPolicy = circuitBreaker(handleAll, { + halfOpenAfter: circuitBreakDuration, + breaker: new ConsecutiveBreaker(maximumConsecutiveFailures), + }); + this.#tokenPricePolicy = wrap(retryPolicy, circuitBreakerPolicy); + } + /** * Retrieves prices in the given currency for the tokens identified by the * given addresses which are expected to live on the given chain. @@ -275,7 +322,7 @@ export const codefiTokenPricesServiceV2: AbstractTokenPricesService< const pricesByCurrencyByTokenAddress: SpotPricesEndpointData< Lowercase, Lowercase - > = await handleFetch(url); + > = await this.#tokenPricePolicy.execute(() => handleFetch(url)); return tokenAddresses.reduce( ( @@ -312,7 +359,7 @@ export const codefiTokenPricesServiceV2: AbstractTokenPricesService< }, {}, ) as TokenPricesByTokenAddress; - }, + } /** * Type guard for whether the API can return token prices for the given chain @@ -324,7 +371,7 @@ export const codefiTokenPricesServiceV2: AbstractTokenPricesService< validateChainIdSupported(chainId: unknown): chainId is SupportedChainId { const supportedChainIds: readonly string[] = SUPPORTED_CHAIN_IDS; return typeof chainId === 'string' && supportedChainIds.includes(chainId); - }, + } /** * Type guard for whether the API can return token prices in the given @@ -340,5 +387,5 @@ export const codefiTokenPricesServiceV2: AbstractTokenPricesService< typeof currency === 'string' && supportedCurrencies.includes(currency.toLowerCase()) ); - }, -}; + } +} diff --git a/packages/assets-controllers/src/token-prices-service/index.ts b/packages/assets-controllers/src/token-prices-service/index.ts index edb6ff1d1f9..0c0cbd56c84 100644 --- a/packages/assets-controllers/src/token-prices-service/index.ts +++ b/packages/assets-controllers/src/token-prices-service/index.ts @@ -1,2 +1,2 @@ export type { AbstractTokenPricesService } from './abstract-token-prices-service'; -export { codefiTokenPricesServiceV2 } from './codefi-v2'; +export { CodefiTokenPricesServiceV2 } from './codefi-v2'; diff --git a/yarn.lock b/yarn.lock index 00dab9fccc9..da5034445b5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1757,6 +1757,7 @@ __metadata: "@types/node": ^16.18.54 "@types/uuid": ^8.3.0 async-mutex: ^0.2.6 + cockatiel: 3.1.1 deepmerge: ^4.2.2 ethereumjs-util: ^7.0.10 jest: ^27.5.1 @@ -4763,6 +4764,13 @@ __metadata: languageName: node linkType: hard +"cockatiel@npm:3.1.1": + version: 3.1.1 + resolution: "cockatiel@npm:3.1.1" + checksum: c394fa5dc5a0f21a9ff9f007f16320a162000191c570fa277b527a72505a954aae5f2e93b0de0a558f5e3340fed37c014c9fe72d43adfee4aa09d976bdefe745 + languageName: node + linkType: hard + "collect-v8-coverage@npm:^1.0.0": version: 1.0.2 resolution: "collect-v8-coverage@npm:1.0.2"