Skip to content

Commit

Permalink
Refactor services to align with eventual RpcService (#5109)
Browse files Browse the repository at this point in the history
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 any
module that consumes 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`
- Deprecate the `onBreak` and `onDegraded` constructor options, and add
methods instead

Note that we are not including `TokenSearchApiService` in this commit,
even though it is a class, because it does not use the retry or circuit
breaker policy.
  • Loading branch information
mcmire authored Jan 24, 2025
1 parent 763f06f commit 5058fef
Show file tree
Hide file tree
Showing 15 changed files with 543 additions and 1,164 deletions.
2 changes: 1 addition & 1 deletion eslint-warning-thresholds.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": 143,
"import-x/namespace": 189,
"import-x/no-named-as-default": 1,
"import-x/no-named-as-default-member": 8,
Expand Down
11 changes: 11 additions & 0 deletions packages/assets-controllers/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- Add `onBreak` and `onDegraded` methods to `CodefiTokenPricesServiceV2` ([#5109](https://github.com/MetaMask/core/pull/5109))
- 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 ([#5109](https://github.com/MetaMask/core/pull/5109))
- Add `@metamask/controller-utils@^11.4.5` as a dependency ([#5109](https://github.com/MetaMask/core/pull/5109))
- `cockatiel` should still be in the dependency tree because it's now a dependency of `@metamask/controller-utils`

## [46.0.1]

### Changed
Expand Down
1 change: 0 additions & 1 deletion packages/assets-controllers/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
16 changes: 6 additions & 10 deletions packages/assets-controllers/src/TokenRatesController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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(
{
Expand Down
2 changes: 1 addition & 1 deletion packages/assets-controllers/src/TokenRatesController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ export class TokenRatesController extends StaticIntervalPollingController<TokenR

#pollState = PollState.Inactive;

#tokenPricesService: AbstractTokenPricesService;
readonly #tokenPricesService: AbstractTokenPricesService;

#inProcessExchangeRateUpdates: Record<`${Hex}:${string}`, Promise<void>> = {};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { ServicePolicy } from '@metamask/controller-utils';
import type { Hex } from '@metamask/utils';

/**
Expand Down Expand Up @@ -53,7 +54,7 @@ export type AbstractTokenPricesService<
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.
Expand Down
Loading

0 comments on commit 5058fef

Please sign in to comment.