Skip to content

Commit

Permalink
fix: Delete invalid SelectedNetworkController state (#26428)
Browse files Browse the repository at this point in the history
## **Description**

The `SelectedNetworkController` state is cleared if any invalid
`networkConfigurationId`s are found in state. We are seeing reports of
this happening in production in v12.0.1.

The suspected cause is `NetworkController` state corruption. We resolved
a few cases of this in v12.0.1, but for users that were affected by
this, the invalid IDs may have propogated to the
`SelectedNetworkController` state already. That is what this migration
intends to fix.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26428?quickstart=1)

## **Related issues**

Fixes #26309

## **Manual testing steps**

We don't have clear reproduction steps for the bug itself. To
artificially reproduce the scenario by changing extension state, this
should work:

* Create a dev build from v12.0.2
* Install the dev build from the `dist/chrome` directory and proceed
through onboarding
* Visit the test dapp, refresh, and connect to it.
* Run this command in the background console:
  ```
  chrome.storage.local.get(
    null,
    (state) => {

state.data.SelectedNetworkController.domains['https://metamask.github.io']
= '123';
      chrome.storage.local.set(state, () => chrome.runtime.reload());
    }
  );
  ```
  * The linked error should now appear in the console of the popup
* Disable the extension
* Switch to this branch and create a dev build
* Enable and reload the extension
  * The error should no longer appear.

## **Screenshots/Recordings**

N/A

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
Gudahtt committed Aug 14, 2024
1 parent 2106727 commit a38cb00
Show file tree
Hide file tree
Showing 2 changed files with 381 additions and 0 deletions.
255 changes: 255 additions & 0 deletions app/scripts/migrations/120.5.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,255 @@
import { cloneDeep } from 'lodash';
import { migrate, version } from './120.5';

const oldVersion = 120.4;

describe('migration #120.5', () => {
afterEach(() => {
jest.resetAllMocks();
});

it('updates the version metadata', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {},
};

const newStorage = await migrate(cloneDeep(oldStorage));

expect(newStorage.meta).toStrictEqual({ version });
});

it('does nothing if SelectedNetworkController state is not set', async () => {
const oldState = {
NetworkController: {
networkConfigurations: {
123: {},
},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual(oldState);
});

it('deletes the SelectedNetworkController state if it is corrupted', async () => {
const oldState = {
NetworkController: {
networkConfigurations: {
123: {},
},
},
SelectedNetworkController: 'invalid',
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({
NetworkController: {
networkConfigurations: {
123: {},
},
},
});
});

it('deletes the SelectedNetworkController state if it is missing the domains state', async () => {
const oldState = {
NetworkController: {
networkConfigurations: {
123: {},
},
},
SelectedNetworkController: {
somethingElse: {},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({
NetworkController: {
networkConfigurations: {
123: {},
},
},
});
});

it('deletes the SelectedNetworkController state if the domains state is corrupted', async () => {
const oldState = {
NetworkController: {
networkConfigurations: {
123: {},
},
},
SelectedNetworkController: {
domains: 'invalid',
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({
NetworkController: {
networkConfigurations: {
123: {},
},
},
});
});

it('deletes the SelectedNetworkController state if NetworkController state is missing', async () => {
const oldState = {
SelectedNetworkController: {
domains: {},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({});
});

it('deletes the SelectedNetworkController state if NetworkController state is corrupted', async () => {
const oldState = {
NetworkController: 'invalid',
SelectedNetworkController: {
domains: {},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({
NetworkController: 'invalid',
});
});

it('deletes the SelectedNetworkController state if NetworkController has no networkConfigurations', async () => {
const oldState = {
NetworkController: {},
SelectedNetworkController: {
domains: {},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({
NetworkController: {},
});
});

it('deletes the SelectedNetworkController state if NetworkController networkConfigurations state is corrupted', async () => {
const oldState = {
NetworkController: { networkConfigurations: 'invalid' },
SelectedNetworkController: {
domains: {},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({
NetworkController: { networkConfigurations: 'invalid' },
});
});

it('does nothing if SelectedNetworkController domains state is empty', async () => {
const oldState = {
NetworkController: { networkConfigurations: {} },
SelectedNetworkController: {
domains: {},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual(oldState);
});

it('does nothing if SelectedNetworkController domains state is valid', async () => {
const oldState = {
NetworkController: {
networkConfigurations: {
123: {},
},
},
SelectedNetworkController: {
domains: {
'example1.test': '123',
'example2.test': 'mainnet',
'example3.test': 'goerli',
'example4.test': 'sepolia',
'example5.test': 'linea-goerli',
'example6.test': 'linea-sepolia',
'example7.test': 'linea-mainnet',
},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual(oldState);
});

it('deletes the SelectedNetworkController state if an invalid networkConfigurationId is found', async () => {
const oldState = {
NetworkController: {
networkConfigurations: {
123: {},
},
},
SelectedNetworkController: {
domains: {
'domain.test': '456',
},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual({
NetworkController: {
networkConfigurations: {
123: {},
},
},
});
});
});
126 changes: 126 additions & 0 deletions app/scripts/migrations/120.5.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import { hasProperty, isObject } from '@metamask/utils';
import { cloneDeep } from 'lodash';

type VersionedData = {
meta: { version: number };
data: Record<string, unknown>;
};

export const version = 120.5;

/**
* This migration removes invalid network configuration IDs from the SelectedNetworkController.
*
* @param originalVersionedData - Versioned MetaMask extension state, exactly
* what we persist to dist.
* @param originalVersionedData.meta - State metadata.
* @param originalVersionedData.meta.version - The current state version.
* @param originalVersionedData.data - The persisted MetaMask state, keyed by
* controller.
* @returns Updated versioned MetaMask extension state.
*/
export async function migrate(
originalVersionedData: VersionedData,
): Promise<VersionedData> {
const versionedData = cloneDeep(originalVersionedData);
versionedData.meta.version = version;
transformState(versionedData.data);
return versionedData;
}

/**
* A list of InfuraNetworkType values from extension v12.0.1
* This version of the extension uses `@metamask/network-controller@18.1.2`, which in turn uses
* the types from `@metamask/controller-utils@9.1.0`
*
* See https://github.com/MetaMask/core/blob/34542cf6e808f294fd83c7c5f70d1bc7418f8a9e/packages/controller-utils/src/types.ts#L4
*
* Hard-coded here rather than imported so that this migration continues to work correctly as these
* constants get updated in the future.
*/
const infuraNetworkTypes = [
'mainnet',
'goerli',
'sepolia',
'linea-goerli',
'linea-sepolia',
'linea-mainnet',
];

/**
* Remove invalid network configuration IDs from the SelectedNetworkController.
*
* @param state - The persisted MetaMask state, keyed by controller.
*/
function transformState(state: Record<string, unknown>): void {
if (!hasProperty(state, 'SelectedNetworkController')) {
return;
}
if (!isObject(state.SelectedNetworkController)) {
console.error(
`Migration ${version}: Invalid SelectedNetworkController state of type '${typeof state.SelectedNetworkController}'`,
);
delete state.SelectedNetworkController;
return;
} else if (!hasProperty(state.SelectedNetworkController, 'domains')) {
console.error(
`Migration ${version}: Missing SelectedNetworkController domains state`,
);
delete state.SelectedNetworkController;
return;
} else if (!isObject(state.SelectedNetworkController.domains)) {
console.error(
`Migration ${version}: Invalid SelectedNetworkController domains state of type '${typeof state
.SelectedNetworkController.domains}'`,
);
delete state.SelectedNetworkController;
return;
}

if (!hasProperty(state, 'NetworkController')) {
delete state.SelectedNetworkController;
return;
} else if (!isObject(state.NetworkController)) {
console.error(
new Error(
`Migration ${version}: Invalid NetworkController state of type '${typeof state.NetworkController}'`,
),
);
delete state.SelectedNetworkController;
return;
} else if (!hasProperty(state.NetworkController, 'networkConfigurations')) {
delete state.SelectedNetworkController;
return;
} else if (!isObject(state.NetworkController.networkConfigurations)) {
console.error(
new Error(
`Migration ${version}: Invalid NetworkController networkConfigurations state of type '${typeof state.NetworkController}'`,
),
);
delete state.SelectedNetworkController;
return;
}

const validNetworkConfigurationIds = [
...infuraNetworkTypes,
...Object.keys(state.NetworkController.networkConfigurations),
];
const domainMappedNetworkConfigurationIds = Object.values(
state.SelectedNetworkController.domains,
);

for (const configurationId of domainMappedNetworkConfigurationIds) {
if (
typeof configurationId !== 'string' ||
!validNetworkConfigurationIds.includes(configurationId)
) {
console.error(
new Error(
`Migration ${version}: Invalid networkConfigurationId found in SelectedNetworkController state: '${configurationId}'`,
),
);
delete state.SelectedNetworkController;
return;
}
}
}

0 comments on commit a38cb00

Please sign in to comment.