Skip to content

Commit

Permalink
Initialize asset controllers with the current network
Browse files Browse the repository at this point in the history 
Certain controllers in `@metamask/assets-controllers` would default to
assuming the current selected network was Ethereum Mainnet, or they'd
default with an empty string as the `chainId` (causing errors if any
methods were called). Instead they now all get initialized with the
current chain ID, passed in as a new constructor parameter.

Affected controllers are:
* `AssetsContractController`
* `NftController`
* `NftDetectionController`
* `TokenRatesController`
* `TokensController`

Relates to #1209
  • Loading branch information
@Gudahtt
Gudahtt committed May 10, 2023
1 parent 6286a90 commit 76a0b37
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 14 deletions.
2 changes: 2 additions & 0 deletions packages/assets-controllers/src/AssetsContractController.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import HttpProvider from 'ethjs-provider-http';
import {
ChainId,
IPFS_DEFAULT_GATEWAY_URL,
NetworkType,
} from '@metamask/controller-utils';
Expand Down Expand Up @@ -45,6 +46,7 @@ const setupControllers = () => {
});
const preferences = new PreferencesController();
const assetsContract = new AssetsContractController({
chainId: ChainId.mainnet,
onPreferencesStateChange: (listener) => preferences.subscribe(listener),
onNetworkStateChange: (listener) =>
messenger.subscribe('NetworkController:stateChange', listener),
Expand Down
5 changes: 4 additions & 1 deletion packages/assets-controllers/src/AssetsContractController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,19 @@ export class AssetsContractController extends BaseController<
* Creates a AssetsContractController instance.
*
* @param options - The controller options.
* @param options.chainId - The chain ID of the current network.
* @param options.onPreferencesStateChange - Allows subscribing to preference controller state changes.
* @param options.onNetworkStateChange - Allows subscribing to network controller state changes.
* @param config - Initial options used to configure this controller.
* @param state - Initial state to set on this controller.
*/
constructor(
{
chainId: initialChainId,
onPreferencesStateChange,
onNetworkStateChange,
}: {
chainId: string;
onPreferencesStateChange: (
listener: (preferencesState: PreferencesState) => void,
) => void;
Expand All @@ -107,7 +110,7 @@ export class AssetsContractController extends BaseController<
this.defaultConfig = {
provider: undefined,
ipfsGateway: IPFS_DEFAULT_GATEWAY_URL,
chainId: SupportedTokenDetectionNetworks.mainnet,
chainId: initialChainId,
};
this.initialize();

Expand Down
2 changes: 2 additions & 0 deletions packages/assets-controllers/src/NftController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,15 @@ function setupController({
};

const assetsContract = new AssetsContractController({
chainId: ChainId.mainnet,
onPreferencesStateChange: (listener) => preferences.subscribe(listener),
onNetworkStateChange: (listener) =>
onNetworkStateChangeListeners.push(listener),
});
const onNftAddedSpy = includeOnNftAdded ? jest.fn() : undefined;

const nftController = new NftController({
chainId: ChainId.mainnet,
onPreferencesStateChange: (listener) => preferences.subscribe(listener),
onNetworkStateChange: (listener) =>
onNetworkStateChangeListeners.push(listener),
Expand Down
5 changes: 4 additions & 1 deletion packages/assets-controllers/src/NftController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,7 @@ export class NftController extends BaseController<NftConfig, NftState> {
* Creates an NftController instance.
*
* @param options - The controller options.
* @param options.chainId - The chain ID of the current network.
* @param options.onPreferencesStateChange - Allows subscribing to preference controller state changes.
* @param options.onNetworkStateChange - Allows subscribing to network controller state changes.
* @param options.getERC721AssetName - Gets the name of the asset at the given address.
Expand All @@ -872,6 +873,7 @@ export class NftController extends BaseController<NftConfig, NftState> {
*/
constructor(
{
chainId: initialChainId,
onPreferencesStateChange,
onNetworkStateChange,
getERC721AssetName,
Expand All @@ -882,6 +884,7 @@ export class NftController extends BaseController<NftConfig, NftState> {
getERC1155TokenURI,
onNftAdded,
}: {
chainId: string;
onPreferencesStateChange: (
listener: (preferencesState: PreferencesState) => void,
) => void;
Expand All @@ -908,7 +911,7 @@ export class NftController extends BaseController<NftConfig, NftState> {
super(config, state);
this.defaultConfig = {
selectedAddress: '',
chainId: '',
chainId: initialChainId,
ipfsGateway: IPFS_DEFAULT_GATEWAY_URL,
openSeaEnabled: false,
useIPFSSubdomains: true,
Expand Down
7 changes: 6 additions & 1 deletion packages/assets-controllers/src/NftDetectionController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@ describe('NftDetectionController', () => {
beforeEach(async () => {
preferences = new PreferencesController();
assetsContract = new AssetsContractController({
chainId: ChainId.mainnet,
onPreferencesStateChange: (listener) => preferences.subscribe(listener),
onNetworkStateChange: networkStateChangeNoop,
});

nftController = new NftController({
chainId: ChainId.mainnet,
onPreferencesStateChange: (listener) => preferences.subscribe(listener),
onNetworkStateChange: networkStateChangeNoop,
getERC721AssetName:
Expand All @@ -47,6 +49,7 @@ describe('NftDetectionController', () => {
});

nftDetection = new NftDetectionController({
chainId: ChainId.mainnet,
onNftsStateChange: (listener) => nftController.subscribe(listener),
onPreferencesStateChange: (listener) => preferences.subscribe(listener),
onNetworkStateChange: networkStateChangeNoop,
Expand All @@ -55,7 +58,7 @@ describe('NftDetectionController', () => {
getNftState: () => nftController.state,
});

nftController.configure({ chainId: '1', selectedAddress: '0x1' });
nftController.configure({ selectedAddress: '0x1' });
preferences.setOpenSeaEnabled(true);
preferences.setUseNftDetection(true);

Expand Down Expand Up @@ -212,6 +215,7 @@ describe('NftDetectionController', () => {
);
const nftsDetectionController = new NftDetectionController(
{
chainId: ChainId.mainnet,
onNftsStateChange: (listener) => nftController.subscribe(listener),
onPreferencesStateChange: (listener) =>
preferences.subscribe(listener),
Expand Down Expand Up @@ -247,6 +251,7 @@ describe('NftDetectionController', () => {
);
new NftDetectionController(
{
chainId: ChainId.goerli,
onNftsStateChange: (listener) => nftController.subscribe(listener),
onPreferencesStateChange: (listener) =>
preferences.subscribe(listener),
Expand Down
9 changes: 6 additions & 3 deletions packages/assets-controllers/src/NftDetectionController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export interface ApiNftCreator {
*/
export interface NftDetectionConfig extends BaseConfig {
interval: number;
chainId: `0x${string}` | `${number}` | number;
chainId: string;
selectedAddress: string;
}

Expand Down Expand Up @@ -204,6 +204,7 @@ export class NftDetectionController extends BaseController<
* Creates an NftDetectionController instance.
*
* @param options - The controller options.
* @param options.chainId - The chain ID of the current network.
* @param options.onNftsStateChange - Allows subscribing to assets controller state changes.
* @param options.onPreferencesStateChange - Allows subscribing to preferences controller state changes.
* @param options.onNetworkStateChange - Allows subscribing to network controller state changes.
Expand All @@ -215,12 +216,14 @@ export class NftDetectionController extends BaseController<
*/
constructor(
{
chainId: initialChainId,
onPreferencesStateChange,
onNetworkStateChange,
getOpenSeaApiKey,
addNft,
getNftState,
}: {
chainId: string;
onNftsStateChange: (listener: (nftsState: NftState) => void) => void;
onPreferencesStateChange: (
listener: (preferencesState: PreferencesState) => void,
Expand All @@ -238,7 +241,7 @@ export class NftDetectionController extends BaseController<
super(config, state);
this.defaultConfig = {
interval: DEFAULT_INTERVAL,
chainId: '1',
chainId: initialChainId,
selectedAddress: '',
disabled: true,
};
Expand Down Expand Up @@ -266,7 +269,7 @@ export class NftDetectionController extends BaseController<

onNetworkStateChange(({ providerConfig }) => {
this.configure({
chainId: providerConfig.chainId as NftDetectionConfig['chainId'],
chainId: providerConfig.chainId,
});
});
this.getOpenSeaApiKey = getOpenSeaApiKey;
Expand Down
24 changes: 19 additions & 5 deletions packages/assets-controllers/src/TokenRatesController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ describe('TokenRatesController', () => {

it('should set default state', () => {
const controller = new TokenRatesController({
chainId: '1',
onTokensStateChange: sinon.stub(),
onCurrencyRateStateChange: sinon.stub(),
onNetworkStateChange: sinon.stub(),
Expand All @@ -129,6 +130,7 @@ describe('TokenRatesController', () => {

it('should initialize with the default config', () => {
const controller = new TokenRatesController({
chainId: '1',
onTokensStateChange: sinon.stub(),
onCurrencyRateStateChange: sinon.stub(),
onNetworkStateChange: sinon.stub(),
Expand All @@ -137,14 +139,15 @@ describe('TokenRatesController', () => {
disabled: false,
interval: 180000,
nativeCurrency: 'eth',
chainId: '',
chainId: '1',
tokens: [],
threshold: 21600000,
});
});

it('should throw when tokens property is accessed', () => {
const controller = new TokenRatesController({
chainId: '1',
onTokensStateChange: sinon.stub(),
onCurrencyRateStateChange: sinon.stub(),
onNetworkStateChange: sinon.stub(),
Expand All @@ -160,6 +163,7 @@ describe('TokenRatesController', () => {
const times = 5;
new TokenRatesController(
{
chainId: '1',
onTokensStateChange: jest.fn(),
onCurrencyRateStateChange: jest.fn(),
onNetworkStateChange: jest.fn(),
Expand All @@ -182,6 +186,7 @@ describe('TokenRatesController', () => {
it('should not update rates if disabled', async () => {
const controller = new TokenRatesController(
{
chainId: '1',
onTokensStateChange: sinon.stub(),
onCurrencyRateStateChange: sinon.stub(),
onNetworkStateChange: sinon.stub(),
Expand All @@ -200,6 +205,7 @@ describe('TokenRatesController', () => {
const mock = sinon.stub(global, 'clearTimeout');
const controller = new TokenRatesController(
{
chainId: '1',
onTokensStateChange: sinon.stub(),
onCurrencyRateStateChange: sinon.stub(),
onNetworkStateChange: sinon.stub(),
Expand All @@ -223,19 +229,21 @@ describe('TokenRatesController', () => {
});
const preferences = new PreferencesController();
const tokensController = new TokensController({
chainId: '1',
onPreferencesStateChange: (listener) => preferences.subscribe(listener),
onNetworkStateChange: (listener) =>
messenger.subscribe('NetworkController:stateChange', listener),
messenger: undefined as unknown as TokensControllerMessenger,
});
const controller = new TokenRatesController(
{
chainId: '1',
onTokensStateChange: (listener) => tokensController.subscribe(listener),
onCurrencyRateStateChange: sinon.stub(),
onNetworkStateChange: (listener) =>
messenger.subscribe('NetworkController:stateChange', listener),
},
{ interval: 10, chainId: '1' },
{ interval: 10 },
);
const address = '0x89d24A6b4CcB1B6fAA2625fE562bDD9a23260359';
expect(controller.state.contractExchangeRates).toStrictEqual({});
Expand All @@ -257,6 +265,7 @@ describe('TokenRatesController', () => {
it('should handle balance not found in API', async () => {
const controller = new TokenRatesController(
{
chainId: '1',
onTokensStateChange: sinon.stub(),
onCurrencyRateStateChange: sinon.stub(),
onNetworkStateChange: sinon.stub(),
Expand Down Expand Up @@ -285,6 +294,7 @@ describe('TokenRatesController', () => {
const onNetworkStateChange = sinon.stub();
const controller = new TokenRatesController(
{
chainId: '1',
onTokensStateChange,
onCurrencyRateStateChange,
onNetworkStateChange,
Expand All @@ -311,6 +321,7 @@ describe('TokenRatesController', () => {
const onNetworkStateChange = sinon.stub();
const controller = new TokenRatesController(
{
chainId: '1',
onTokensStateChange,
onCurrencyRateStateChange,
onNetworkStateChange,
Expand Down Expand Up @@ -360,13 +371,14 @@ describe('TokenRatesController', () => {
const onNetworkStateChange = sinon.stub();
const controller = new TokenRatesController(
{
chainId: '137',
onTokensStateChange,
onCurrencyRateStateChange: sinon.stub(),
onNetworkStateChange,
},
{ interval: 10 },
);
await controller.configure({ chainId: '137', nativeCurrency: 'MATIC' });
await controller.configure({ nativeCurrency: 'MATIC' });

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await tokenStateChangeListener!({
Expand Down Expand Up @@ -422,14 +434,15 @@ describe('TokenRatesController', () => {

const controller = new TokenRatesController(
{
chainId: '1',
onTokensStateChange,
onNetworkStateChange,
onCurrencyRateStateChange: sinon.stub(),
},
{ interval: 10 },
);

await controller.configure({ chainId: '1', nativeCurrency: 'ETH' });
await controller.configure({ nativeCurrency: 'ETH' });

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await tokenStateChangeListener!({
Expand Down Expand Up @@ -494,11 +507,12 @@ describe('TokenRatesController', () => {

const controller = new TokenRatesController(
{
chainId: '1',
onTokensStateChange,
onNetworkStateChange: sinon.stub(),
onCurrencyRateStateChange: sinon.stub(),
},
{ interval: 10, chainId: '1', nativeCurrency: 'ETH' },
{ interval: 10, nativeCurrency: 'ETH' },
);

expect(controller.state.contractExchangeRates).toStrictEqual({});
Expand Down
5 changes: 4 additions & 1 deletion packages/assets-controllers/src/TokenRatesController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ export class TokenRatesController extends BaseController<
* Creates a TokenRatesController instance.
*
* @param options - The controller options.
* @param options.chainId - The chain ID of the current network.
* @param options.onTokensStateChange - Allows subscribing to token controller state changes.
* @param options.onCurrencyRateStateChange - Allows subscribing to currency rate controller state changes.
* @param options.onNetworkStateChange - Allows subscribing to network state changes.
Expand All @@ -172,10 +173,12 @@ export class TokenRatesController extends BaseController<
*/
constructor(
{
chainId: initialChainId,
onTokensStateChange,
onCurrencyRateStateChange,
onNetworkStateChange,
}: {
chainId: string;
onTokensStateChange: (
listener: (tokensState: TokensState) => void,
) => void;
Expand All @@ -194,7 +197,7 @@ export class TokenRatesController extends BaseController<
disabled: false,
interval: 3 * 60 * 1000,
nativeCurrency: 'eth',
chainId: '',
chainId: initialChainId,
tokens: [],
threshold: 6 * 60 * 60 * 1000,
};
Expand Down
2 changes: 1 addition & 1 deletion packages/assets-controllers/src/TokensController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,11 @@ describe('TokensController', () => {
const defaultSelectedAddress = '0x1';
preferences = new PreferencesController();
tokensController = new TokensController({
chainId: ChainId.mainnet,
onPreferencesStateChange: (listener) => preferences.subscribe(listener),
onNetworkStateChange: (listener) =>
(onNetworkStateChangeListener = listener),
config: {
chainId: ChainId.mainnet,
selectedAddress: defaultSelectedAddress,
},
messenger,
Expand Down
Loading

0 comments on commit 76a0b37

Please sign in to comment.