Skip to content

Commit

Permalink
Replace NetworkController w/ core version
Browse files Browse the repository at this point in the history
This commit fulfills a long-standing desire to get the extension using
the same network controller as mobile by removing NetworkController from
this repo and replacing it with NetworkController from the
`@metamask/network-controller` package.

The new version of NetworkController is different the old one in a few
ways:

- The new controller inherits from BaseControllerV2, so the `state`
  property is used to access the state instead of `store.getState()`.
  All references of the latter have been replaced with the former.
- As the new controller no longer has a `store` property, it cannot be
  subscribed to; the controller takes a messenger which can be
  subscribed to instead. There were various places within
  MetamaskController where the old way of subscribing has been replaced
  with the new way. In addition, DetectTokensController has been updated
  to take a messenger object so that it can listen for NetworkController
  state changes.
- The state of the new controller is not updatable from the outside.
  This affected BackupController, which dumps state from
  NetworkController (among other controllers), but also loads the same
  state into NetworkController on import. A method `loadBackup` has been
  added to NetworkController to facilitate this use case, and
  BackupController is now using this method instead of attempting to
  call `update` on NetworkController.
- The new controller does not have a `getCurrentChainId` method;
  instead, the chain ID can be read from the provider config in state.
  This affected MmiController. (MmiController was also updated to read
  custom networks from the new network controller instead of the
  preferences controller).
- The default network that the new controller is set to is always
  Mainnet (previously it could be either localhost or Goerli in test
  mode, depending on environment variables).
  • Loading branch information
mcmire committed Jun 15, 2023
1 parent c16b35c commit c658295
Show file tree
Hide file tree
Showing 29 changed files with 207 additions and 13,464 deletions.
4 changes: 0 additions & 4 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ module.exports = {
excludedFiles: [
'app/scripts/controllers/app-state.test.js',
'app/scripts/controllers/mmi-controller.test.js',
'app/scripts/controllers/network/**/*.test.js',
'app/scripts/controllers/permissions/**/*.test.js',
'app/scripts/lib/**/*.test.js',
'app/scripts/migrations/*.test.js',
Expand Down Expand Up @@ -267,9 +266,6 @@ module.exports = {
'**/__snapshots__/*.snap',
'app/scripts/controllers/app-state.test.js',
'app/scripts/controllers/mmi-controller.test.js',
'app/scripts/controllers/network/**/*.test.js',
'app/scripts/controllers/network/**/*.test.ts',
'app/scripts/controllers/network/provider-api-tests/*.ts',
'app/scripts/controllers/permissions/**/*.test.js',
'app/scripts/lib/**/*.test.js',
'app/scripts/migrations/*.test.js',
Expand Down
1 change: 0 additions & 1 deletion .mocharc.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ module.exports = {
'./app/scripts/migrations/*.test.js',
'./app/scripts/platforms/*.test.js',
'./app/scripts/controllers/app-state.test.js',
'./app/scripts/controllers/network/**/*.test.js',
'./app/scripts/controllers/permissions/**/*.test.js',
'./app/scripts/controllers/mmi-controller.test.js',
'./app/scripts/constants/error-utils.test.js',
Expand Down
2 changes: 1 addition & 1 deletion app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ export function setupController(

setupEnsIpfsResolver({
getCurrentChainId: () =>
controller.networkController.store.getState().providerConfig.chainId,
controller.networkController.state.providerConfig.chainId,
getIpfsGateway: controller.preferencesController.getIpfsGateway.bind(
controller.preferencesController,
),
Expand Down
4 changes: 2 additions & 2 deletions app/scripts/controllers/backup.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export default class BackupController {
}

if (network) {
this.networkController.store.updateState(network);
this.networkController.loadBackup(network);
}

if (preferences || addressBook || network) {
Expand All @@ -48,7 +48,7 @@ export default class BackupController {
addressBook: { ...this.addressBookController.state },
network: {
networkConfigurations:
this.networkController.store.getState().networkConfigurations,
this.networkController.state.networkConfigurations,
},
};

Expand Down
21 changes: 9 additions & 12 deletions app/scripts/controllers/backup.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,15 @@ function getMockAddressBookController() {
}

function getMockNetworkController() {
const mcState = {
const state = {
networkConfigurations: {},

update: (store) => (mcState.store = store),
};

mcState.store = {
getState: sinon.stub().returns(mcState),
updateState: (store) => (mcState.store = store),
const loadBackup = ({ networkConfigurations }) => {
Object.assign(state, { networkConfigurations });
};

return mcState;
return { state, loadBackup };
}

const jsonData = JSON.stringify({
Expand Down Expand Up @@ -174,28 +171,28 @@ describe('BackupController', function () {

it('should restore backup', async function () {
const backupController = getBackupController();
backupController.restoreUserData(jsonData);
await backupController.restoreUserData(jsonData);
// check networks backup
assert.equal(
backupController.networkController.store.networkConfigurations[
backupController.networkController.state.networkConfigurations[
'network-configuration-id-1'
].chainId,
'0x539',
);
assert.equal(
backupController.networkController.store.networkConfigurations[
backupController.networkController.state.networkConfigurations[
'network-configuration-id-2'
].chainId,
'0x38',
);
assert.equal(
backupController.networkController.store.networkConfigurations[
backupController.networkController.state.networkConfigurations[
'network-configuration-id-3'
].chainId,
'0x61',
);
assert.equal(
backupController.networkController.store.networkConfigurations[
backupController.networkController.state.networkConfigurations[
'network-configuration-id-4'
].chainId,
'0x89',
Expand Down
35 changes: 14 additions & 21 deletions app/scripts/controllers/detect-tokens.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@ export default class DetectTokensController {
* @param config.tokensController
* @param config.assetsContractController
* @param config.trackMetaMetricsEvent
* @param config.messenger
*/
constructor({
messenger,
interval = DEFAULT_INTERVAL,
preferences,
network,
Expand All @@ -44,6 +46,7 @@ export default class DetectTokensController {
assetsContractController = null,
trackMetaMetricsEvent,
} = {}) {
this.messenger = messenger;
this.assetsContractController = assetsContractController;
this.tokensController = tokensController;
this.preferences = preferences;
Expand All @@ -59,7 +62,7 @@ export default class DetectTokensController {
});
this.hiddenTokens = this.tokensController?.state.ignoredTokens;
this.detectedTokens = this.tokensController?.state.detectedTokens;
this.chainId = this.getChainIdFromNetworkStore(network);
this.chainId = this.getChainIdFromNetworkStore();
this._trackMetaMetricsEvent = trackMetaMetricsEvent;

preferences?.store.subscribe(({ selectedAddress, useTokenDetection }) => {
Expand All @@ -81,6 +84,13 @@ export default class DetectTokensController {
this.detectedTokens = detectedTokens;
},
);
messenger.subscribe('NetworkController:stateChange', () => {
if (this.chainId !== this.getChainIdFromNetworkStore()) {
const chainId = this.getChainIdFromNetworkStore();
this.chainId = chainId;
this.restartTokenDetection({ chainId: this.chainId });
}
});
}

/**
Expand All @@ -93,7 +103,7 @@ export default class DetectTokensController {
async detectNewTokens({ selectedAddress, chainId } = {}) {
const addressAgainstWhichToDetect = selectedAddress ?? this.selectedAddress;
const chainIdAgainstWhichToDetect =
chainId ?? this.getChainIdFromNetworkStore(this._network);
chainId ?? this.getChainIdFromNetworkStore();
if (!this.isActive) {
return;
}
Expand Down Expand Up @@ -208,8 +218,8 @@ export default class DetectTokensController {
this.interval = DEFAULT_INTERVAL;
}

getChainIdFromNetworkStore(network) {
return network?.store.getState().providerConfig.chainId;
getChainIdFromNetworkStore() {
return this.network?.state.providerConfig.chainId;
}

/* eslint-disable accessor-pairs */
Expand All @@ -226,23 +236,6 @@ export default class DetectTokensController {
}, interval);
}

/**
* @type {object}
*/
set network(network) {
if (!network) {
return;
}
this._network = network;
this._network.store.subscribe(() => {
if (this.chainId !== this.getChainIdFromNetworkStore(network)) {
const chainId = this.getChainIdFromNetworkStore(network);
this.chainId = chainId;
this.restartTokenDetection({ chainId: this.chainId });
}
});
}

/**
* In setter when isUnlocked is updated to true, detectNewTokens and restart polling
*
Expand Down
38 changes: 25 additions & 13 deletions app/scripts/controllers/detect-tokens.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,19 @@ import {
AssetsContractController,
} from '@metamask/assets-controllers';
import { toHex } from '@metamask/controller-utils';
import { NetworkController } from '@metamask/network-controller';
import { NETWORK_TYPES } from '../../../shared/constants/network';
import { toChecksumHexAddress } from '../../../shared/modules/hexstring-utils';
import DetectTokensController from './detect-tokens';
import { NetworkController } from './network';
import PreferencesController from './preferences';

function buildMessenger() {
return new ControllerMessenger().getRestricted({
name: 'DetectTokensController',
allowedEvents: ['NetworkController:stateChange'],
});
}

describe('DetectTokensController', function () {
let sandbox,
assetsContractController,
Expand Down Expand Up @@ -230,23 +237,20 @@ describe('DetectTokensController', function () {
onPreferencesStateChange: preferences.store.subscribe.bind(
preferences.store,
),
onNetworkStateChange: (cb) =>
network.store.subscribe((networkState) => {
const modifiedNetworkState = {
...networkState,
providerConfig: {
...networkState.providerConfig,
},
};
return cb(modifiedNetworkState);
}),
onNetworkStateChange: networkControllerMessenger.subscribe.bind(
networkControllerMessenger,
'NetworkController:stateChange',
),
});

assetsContractController = new AssetsContractController({
onPreferencesStateChange: preferences.store.subscribe.bind(
preferences.store,
),
onNetworkStateChange: network.store.subscribe.bind(network.store),
onNetworkStateChange: networkControllerMessenger.subscribe.bind(
networkControllerMessenger,
'NetworkController:stateChange',
),
});
});

Expand All @@ -257,7 +261,7 @@ describe('DetectTokensController', function () {

it('should poll on correct interval', async function () {
const stub = sinon.stub(global, 'setInterval');
new DetectTokensController({ interval: 1337 }); // eslint-disable-line no-new
new DetectTokensController({ messenger: buildMessenger(), interval: 1337 }); // eslint-disable-line no-new
assert.strictEqual(stub.getCall(0).args[1], 1337);
stub.restore();
});
Expand All @@ -266,6 +270,7 @@ describe('DetectTokensController', function () {
const clock = sandbox.useFakeTimers();
await network.setProviderType(NETWORK_TYPES.MAINNET);
const controller = new DetectTokensController({
messenger: buildMessenger(),
preferences,
network,
keyringMemStore,
Expand Down Expand Up @@ -302,6 +307,7 @@ describe('DetectTokensController', function () {
});
await tokenListController.start();
const controller = new DetectTokensController({
messenger: buildMessenger(),
preferences,
network,
keyringMemStore,
Expand All @@ -325,6 +331,7 @@ describe('DetectTokensController', function () {
sandbox.useFakeTimers();
await network.setProviderType(NETWORK_TYPES.MAINNET);
const controller = new DetectTokensController({
messenger: buildMessenger(),
preferences,
network,
keyringMemStore,
Expand Down Expand Up @@ -376,6 +383,7 @@ describe('DetectTokensController', function () {
sandbox.useFakeTimers();
await network.setProviderType(NETWORK_TYPES.MAINNET);
const controller = new DetectTokensController({
messenger: buildMessenger(),
preferences,
network,
keyringMemStore,
Expand Down Expand Up @@ -434,6 +442,7 @@ describe('DetectTokensController', function () {
it('should trigger detect new tokens when change address', async function () {
sandbox.useFakeTimers();
const controller = new DetectTokensController({
messenger: buildMessenger(),
preferences,
network,
keyringMemStore,
Expand All @@ -453,6 +462,7 @@ describe('DetectTokensController', function () {
it('should trigger detect new tokens when submit password', async function () {
sandbox.useFakeTimers();
const controller = new DetectTokensController({
messenger: buildMessenger(),
preferences,
network,
keyringMemStore,
Expand All @@ -471,6 +481,7 @@ describe('DetectTokensController', function () {
const clock = sandbox.useFakeTimers();
await network.setProviderType(NETWORK_TYPES.MAINNET);
const controller = new DetectTokensController({
messenger: buildMessenger(),
preferences,
network,
keyringMemStore,
Expand All @@ -492,6 +503,7 @@ describe('DetectTokensController', function () {
const clock = sandbox.useFakeTimers();
await network.setProviderType(NETWORK_TYPES.MAINNET);
const controller = new DetectTokensController({
messenger: buildMessenger(),
preferences,
network,
keyringMemStore,
Expand Down
Loading

0 comments on commit c658295

Please sign in to comment.