Skip to content

Commit

Permalink
Migrate CurrencyRateController to BaseControllerV2 (#372)
Browse files Browse the repository at this point in the history
The CurrencyRateController has been migrated to the new base
controller. The configuration can now only be set during construction,
as it was never changed at runtime in practice with the old controller.
Similarly, the `disable` function was removed as it wasn't relied upon
in practice.

One major change is that the controller doesn't poll after being
constructed. The "start" method must be called for polling to start.
The "start" and "stop" methods have been added to loosely follow the
conventions used in `metamask-extension` to allow starting and stopping
polling for the purpose of reducing network traffic.

A few tests required substantial updates because of this change. The
ComposableController tests were easiest to fix by deleting the use of
the CurrencyRateController completely, since that test was intended to
test BaseController-based controllers specifically.

The last TokenRatesController test managed to mark the state change
handlers in that controller as 'covered' without actually asserting
anything related to that functionality. That broken test was split into
two pieces, one for each state change handler. Those two tests still
rely upon mocking a would-be private function, which isn't great, but
it matches the patterns used elsewhere in that test module. To improve
it would take a great deal of work, enough for a separate PR.
  • Loading branch information
Gudahtt authored and MajorLift committed Oct 11, 2023
1 parent 1297444 commit 6def841
Show file tree
Hide file tree
Showing 4 changed files with 345 additions and 267 deletions.
32 changes: 0 additions & 32 deletions src/ComposableController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@ import {
RestrictedControllerMessenger,
} from './ControllerMessenger';
import PreferencesController from './user/PreferencesController';
import TokenRatesController from './assets/TokenRatesController';
import { AssetsController } from './assets/AssetsController';
import {
NetworkController,
NetworksChainId,
} from './network/NetworkController';
import { AssetsContractController } from './assets/AssetsContractController';
import CurrencyRateController from './assets/CurrencyRateController';

// Mock BaseControllerV2 classes

Expand Down Expand Up @@ -107,21 +105,13 @@ describe('ComposableController', () => {
assetContractController,
),
});
const currencyRateController = new CurrencyRateController();
const controller = new ComposableController([
new AddressBookController(),
assetController,
assetContractController,
new EnsController(),
currencyRateController,
networkController,
preferencesController,
new TokenRatesController({
onAssetsStateChange: (listener) =>
assetController.subscribe(listener),
onCurrencyRateStateChange: (listener) =>
currencyRateController.subscribe(listener),
}),
]);
expect(controller.state).toStrictEqual({
AddressBookController: { addressBook: {} },
Expand All @@ -137,13 +127,6 @@ describe('ComposableController', () => {
suggestedAssets: [],
tokens: [],
},
CurrencyRateController: {
conversionDate: 0,
conversionRate: 0,
currentCurrency: 'usd',
nativeCurrency: 'ETH',
usdConversionRate: 0,
},
EnsController: {
ensEntries: {},
},
Expand All @@ -159,7 +142,6 @@ describe('ComposableController', () => {
lostIdentities: {},
selectedAddress: '',
},
TokenRatesController: { contractExchangeRates: {} },
});
});

Expand All @@ -182,21 +164,13 @@ describe('ComposableController', () => {
assetContractController,
),
});
const currencyRateController = new CurrencyRateController();
const controller = new ComposableController([
new AddressBookController(),
assetController,
assetContractController,
new EnsController(),
currencyRateController,
networkController,
preferencesController,
new TokenRatesController({
onAssetsStateChange: (listener) =>
assetController.subscribe(listener),
onCurrencyRateStateChange: (listener) =>
currencyRateController.subscribe(listener),
}),
]);
expect(controller.flatState).toStrictEqual({
addressBook: {},
Expand All @@ -205,10 +179,6 @@ describe('ComposableController', () => {
allTokens: {},
collectibleContracts: [],
collectibles: [],
contractExchangeRates: {},
conversionDate: 0,
conversionRate: 0,
currentCurrency: 'usd',
ensEntries: {},
featureFlags: {},
frequentRpcList: [],
Expand All @@ -217,13 +187,11 @@ describe('ComposableController', () => {
ignoredTokens: [],
ipfsGateway: 'https://ipfs.io/ipfs/',
lostIdentities: {},
nativeCurrency: 'ETH',
network: 'loading',
provider: { type: 'mainnet', chainId: NetworksChainId.mainnet },
selectedAddress: '',
suggestedAssets: [],
tokens: [],
usdConversionRate: 0,
});
});

Expand Down
Loading

0 comments on commit 6def841

Please sign in to comment.