Refactor services to align with eventual RpcService

In a future commit, we are adding an RpcService class to
`network-controller` to handle automatic failovers. This class is
different from the existing service classes in a couple of ways:

- RpcService will make use of a new `createServicePolicy` function which
  encapsulates retry and circuit breaker logic. This will relieve the
  consuming module for RpcService from needing to test this logic.
- RpcService will have `onBreak` and `onDegraded` methods instead of
  taking `onBreak` and `onDegraded` callbacks as constructor options.
  This is reflected not only in the class but also in the abstract RPC
  service interface.

To these ends, this commit makes the following changes to
`CodefiTokenPricesServiceV2` in `assets-controllers` and
`ClientConfigApiService` in `remote-feature-flag-controller`:

- Refactor service class to use `createServicePolicy`
- Update the abstract service class interface to extend `ServicePolicy`
  from `controller-utils`, and prepend `I` to the type to indicate that
  it's not a superclass, but an interface
- Deprecate the `onBreak` and `onDegraded` constructor options, and add
  methods instead

eslint-warning-thresholds.json

@@ -5,7 +5,7 @@
   "@typescript-eslint/no-unsafe-enum-comparison": 34,
   "@typescript-eslint/no-unused-vars": 41,
   "@typescript-eslint/prefer-promise-reject-errors": 33,
-  "@typescript-eslint/prefer-readonly": 147,
+  "@typescript-eslint/prefer-readonly": 144,
   "import-x/namespace": 189,
   "import-x/no-named-as-default": 1,
   "import-x/no-named-as-default-member": 8,
@@ -19,7 +19,7 @@
   "n/no-unsupported-features/node-builtins": 4,
   "n/prefer-global/text-encoder": 4,
   "n/prefer-global/text-decoder": 4,
-  "prettier/prettier": 116,
+  "prettier/prettier": 115,
   "promise/always-return": 3,
   "promise/catch-or-return": 2,
   "promise/param-names": 8,

packages/assets-controllers/CHANGELOG.md

@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add `onBreak` and `onDegraded` methods to `CodefiTokenPricesServiceV2`
+  - These serve the same purpose as the `onBreak` and `onDegraded` constructor options, but align more closely with the Cockatiel policy API.
+
+### Changed
+
+- Deprecate `ClientConfigApiService` constructor options `onBreak` and `onDegraded` in favor of methods
+
 ## [46.0.1]
 
 ### Changed

packages/assets-controllers/package.json

@@ -68,7 +68,6 @@
     "async-mutex": "^0.5.0",
     "bitcoin-address-validation": "^2.2.3",
     "bn.js": "^5.2.1",
-    "cockatiel": "^3.1.2",
     "immer": "^9.0.6",
     "lodash": "^4.17.21",
     "multiformats": "^13.1.0",

packages/assets-controllers/src/TokenRatesController.test.ts

@@ -30,7 +30,7 @@ import {
 } from '../../network-controller/tests/helpers';
 import { TOKEN_PRICES_BATCH_SIZE } from './assetsUtil';
 import type {
-  AbstractTokenPricesService,
+  IAbstractTokenPricesService,
   TokenPrice,
   TokenPricesByTokenAddress,
 } from './token-prices-service/abstract-token-prices-service';
@@ -2093,11 +2093,9 @@ describe('TokenRatesController', () => {
               price: 0.002,
             },
           }),
-          validateCurrencySupported: jest.fn().mockReturnValue(
-            false,
-            // Cast used because this method has an assertion in the return
-            // value that I don't know how to type properly with Jest's mock.
-          ) as unknown as AbstractTokenPricesService['validateCurrencySupported'],
+          validateCurrencySupported(_currency: unknown): _currency is string {
+            return false;
+          },
         });
         nock('https://min-api.cryptocompare.com')
           .get('/data/price')
@@ -2288,11 +2286,9 @@ describe('TokenRatesController', () => {
               value: 0.002,
             },
           }),
-          validateChainIdSupported: jest.fn().mockReturnValue(
-            false,
-            // Cast used because this method has an assertion in the return
-            // value that I don't know how to type properly with Jest's mock.
-          ) as unknown as AbstractTokenPricesService['validateChainIdSupported'],
+          validateChainIdSupported(_chainId: unknown): _chainId is Hex {
+            return false;
+          },
         });
         await withController(
           {
@@ -2758,8 +2754,8 @@ async function callUpdateExchangeRatesMethod({
  * @returns The built mock token prices service.
  */
 function buildMockTokenPricesService(
-  overrides: Partial<AbstractTokenPricesService> = {},
-): AbstractTokenPricesService {
+  overrides: Partial<IAbstractTokenPricesService> = {},
+): IAbstractTokenPricesService {
   return {
     async fetchTokenPrices() {
       return {};

packages/assets-controllers/src/TokenRatesController.ts

@@ -26,7 +26,7 @@ import { isEqual } from 'lodash';
 
 import { reduceInBatchesSerially, TOKEN_PRICES_BATCH_SIZE } from './assetsUtil';
 import { fetchExchangeRate as fetchNativeCurrencyExchangeRate } from './crypto-compare-service';
-import type { AbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service';
+import type { IAbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service';
 import { getNativeTokenAddress } from './token-prices-service/codefi-v2';
 import type {
   TokensControllerGetStateAction,
@@ -238,7 +238,7 @@ export class TokenRatesController extends StaticIntervalPollingController<TokenR
 
   #pollState = PollState.Inactive;
 
-  #tokenPricesService: AbstractTokenPricesService;
+  readonly #tokenPricesService: IAbstractTokenPricesService;
 
   #inProcessExchangeRateUpdates: Record<`${Hex}:${string}`, Promise<void>> = {};
 
@@ -275,7 +275,7 @@ export class TokenRatesController extends StaticIntervalPollingController<TokenR
   }: {
     interval?: number;
     disabled?: boolean;
-    tokenPricesService: AbstractTokenPricesService;
+    tokenPricesService: IAbstractTokenPricesService;
     messenger: TokenRatesControllerMessenger;
     state?: Partial<TokenRatesControllerState>;
   }) {
@@ -687,7 +687,7 @@ export class TokenRatesController extends StaticIntervalPollingController<TokenR
     let contractNativeInformations;
     const tokenPricesByTokenAddress = await reduceInBatchesSerially<
       Hex,
-      Awaited<ReturnType<AbstractTokenPricesService['fetchTokenPrices']>>
+      Awaited<ReturnType<IAbstractTokenPricesService['fetchTokenPrices']>>
     >({
       values: [...tokenAddresses].sort(),
       batchSize: TOKEN_PRICES_BATCH_SIZE,

packages/assets-controllers/src/assetsUtil.test.ts

@@ -10,7 +10,7 @@ import { add0x, type Hex } from '@metamask/utils';
 import * as assetsUtil from './assetsUtil';
 import { TOKEN_PRICES_BATCH_SIZE } from './assetsUtil';
 import type { Nft, NftMetadata } from './NftController';
-import type { AbstractTokenPricesService } from './token-prices-service';
+import type { IAbstractTokenPricesService } from './token-prices-service';
 
 const DEFAULT_IPFS_URL_FORMAT = 'ipfs://';
 const ALTERNATIVE_IPFS_URL_FORMAT = 'ipfs://ipfs/';
@@ -772,7 +772,7 @@ function buildAddress(number: number) {
  *
  * @returns The mocked functions of token prices service.
  */
-function createMockPriceService(): AbstractTokenPricesService {
+function createMockPriceService(): IAbstractTokenPricesService {
   return {
     validateChainIdSupported(_chainId: unknown): _chainId is Hex {
       return true;

packages/assets-controllers/src/assetsUtil.ts

@@ -8,7 +8,7 @@ import { remove0x } from '@metamask/utils';
 import BN from 'bn.js';
 
 import type { Nft, NftMetadata } from './NftController';
-import type { AbstractTokenPricesService } from './token-prices-service';
+import type { IAbstractTokenPricesService } from './token-prices-service';
 import { type ContractExchangeRates } from './TokenRatesController';
 
 /**
@@ -387,7 +387,7 @@ export async function fetchTokenContractExchangeRates({
   tokenAddresses,
   chainId,
 }: {
-  tokenPricesService: AbstractTokenPricesService;
+  tokenPricesService: IAbstractTokenPricesService;
   nativeCurrency: string;
   tokenAddresses: Hex[];
   chainId: Hex;
@@ -403,7 +403,7 @@ export async function fetchTokenContractExchangeRates({
 
   const tokenPricesByTokenAddress = await reduceInBatchesSerially<
     Hex,
-    Awaited<ReturnType<AbstractTokenPricesService['fetchTokenPrices']>>
+    Awaited<ReturnType<IAbstractTokenPricesService['fetchTokenPrices']>>
   >({
     values: [...tokenAddresses].sort(),
     batchSize: TOKEN_PRICES_BATCH_SIZE,

packages/assets-controllers/src/token-prices-service/abstract-token-prices-service.ts

@@ -1,3 +1,4 @@
+import type { ServicePolicy } from '@metamask/controller-utils';
 import type { Hex } from '@metamask/utils';
 
 /**
@@ -49,11 +50,11 @@ export type TokenPricesByTokenAddress<
  * @template Currency - A type union of valid arguments for the `currency`
  * argument to `fetchTokenPrices`.
  */
-export type AbstractTokenPricesService<
+export type IAbstractTokenPricesService<
   ChainId extends Hex = Hex,
   TokenAddress extends Hex = Hex,
   Currency extends string = string,
-> = {
+> = Partial<Pick<ServicePolicy, 'onBreak' | 'onDegraded'>> & {
   /**
    * Retrieves prices in the given currency for the tokens identified by the
    * given addresses which are expected to live on the given chain.