Skip to content

Commit

Permalink
chore: remove unused usedNetworks state property from `AppStateCont…
Browse files Browse the repository at this point in the history
…roller` (#28813)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Building on the work done to remove the network modal in
[PR](#28765), this PR
finalizes the process by completely removing the unused `usedNetworks`
state property from `AppStateController`.

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

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

## **Related issues**

Fixes:

## **Manual testing steps**

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] 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).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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
cryptodev-2s authored Nov 29, 2024
1 parent 7d252e9 commit 7143c96
Show file tree
Hide file tree
Showing 21 changed files with 104 additions and 98 deletions.
1 change: 0 additions & 1 deletion app/scripts/constants/sentry-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ export const SENTRY_BACKGROUND_STATE = {
termsOfUseLastAgreed: true,
timeoutMinutes: true,
trezorModel: true,
usedNetworks: true,
},
MultichainBalancesController: {
balances: false,
Expand Down
11 changes: 0 additions & 11 deletions app/scripts/controllers/app-state-controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,17 +374,6 @@ describe('AppStateController', () => {
});
});

describe('setFirstTimeUsedNetwork', () => {
it('updates the array of the first time used networks', () => {
const chainId = '0x1';

appStateController.setFirstTimeUsedNetwork(chainId);
expect(appStateController.store.getState().usedNetworks[chainId]).toBe(
true,
);
});
});

describe('setLastInteractedConfirmationInfo', () => {
it('sets information about last confirmation user has interacted with', () => {
const lastInteractedConfirmationInfo = {
Expand Down
20 changes: 0 additions & 20 deletions app/scripts/controllers/app-state-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ export type AppStateControllerState = {
hadAdvancedGasFeesSetPriorToMigration92_3: boolean;
qrHardware: Json;
nftsDropdownState: Json;
usedNetworks: Record<string, boolean>;
surveyLinkLastClickedOrClosed: number | null;
signatureSecurityAlertResponses: Record<string, SecurityAlertResponse>;
// States used for displaying the changed network toast
Expand Down Expand Up @@ -138,7 +137,6 @@ type AppStateControllerInitState = Partial<
AppStateControllerState,
| 'qrHardware'
| 'nftsDropdownState'
| 'usedNetworks'
| 'surveyLinkLastClickedOrClosed'
| 'signatureSecurityAlertResponses'
| 'switchedNetworkDetails'
Expand Down Expand Up @@ -184,11 +182,6 @@ const getDefaultAppStateControllerState = (
...initState,
qrHardware: {},
nftsDropdownState: {},
usedNetworks: {
'0x1': true,
'0x5': true,
'0x539': true,
},
surveyLinkLastClickedOrClosed: null,
signatureSecurityAlertResponses: {},
switchedNetworkDetails: null,
Expand Down Expand Up @@ -704,19 +697,6 @@ export class AppStateController extends EventEmitter {
});
}

/**
* Updates the array of the first time used networks
*
* @param chainId
*/
setFirstTimeUsedNetwork(chainId: string): void {
const currentState = this.store.getState();
const { usedNetworks } = currentState;
usedNetworks[chainId] = true;

this.store.updateState({ usedNetworks });
}

///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
/**
* Set the interactive replacement token with a url and the old refresh token
Expand Down
2 changes: 0 additions & 2 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -3705,8 +3705,6 @@ export default class MetamaskController extends EventEmitter {
appStateController.setShowNetworkBanner.bind(appStateController),
updateNftDropDownState:
appStateController.updateNftDropDownState.bind(appStateController),
setFirstTimeUsedNetwork:
appStateController.setFirstTimeUsedNetwork.bind(appStateController),
setSwitchedNetworkDetails:
appStateController.setSwitchedNetworkDetails.bind(appStateController),
clearSwitchedNetworkDetails:
Expand Down
62 changes: 62 additions & 0 deletions app/scripts/migrations/134.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { cloneDeep } from 'lodash';
import { migrate, version } from './134';

const oldVersion = 133;

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

const newStorage = await migrate(oldStorage);

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

it('Does nothing if `usedNetworks` is not in the `AppStateController` state', async () => {
const oldState = {
AppStateController: {
timeoutMinutes: 0,
},
};

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

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

it('Removes `usedNetworks` from the `AppStateController` state', async () => {
const oldState: {
AppStateController: {
timeoutMinutes: number;
usedNetworks?: Record<string, boolean>;
};
} = {
AppStateController: {
timeoutMinutes: 0,
usedNetworks: {
'0x1': true,
'0x5': true,
'0x539': true,
},
},
};
const expectedState = {
AppStateController: {
timeoutMinutes: 0,
},
};

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

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

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

export const version = 134;

/**
* This migration removes `usedNetworks` from `AppStateController` state.
*
* @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;
}

function transformState(
state: Record<string, unknown>,
): Record<string, unknown> {
if (
hasProperty(state, 'AppStateController') &&
isObject(state.AppStateController) &&
hasProperty(state.AppStateController, 'usedNetworks')
) {
console.log('Removing usedNetworks from AppStateController');
delete state.AppStateController.usedNetworks;
}
return state;
}
1 change: 1 addition & 0 deletions app/scripts/migrations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ const migrations = [
require('./131'),
require('./132'),
require('./133'),
require('./134'),
];

export default migrations;
5 changes: 0 additions & 5 deletions test/data/mock-send-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,6 @@
"isAccountMenuOpen": false,
"isUnlocked": true,
"completedOnboarding": true,
"usedNetworks": {
"0x1": true,
"0x5": true,
"0x539": true
},
"showTestnetMessageInDropdown": true,
"alertEnabledness": {
"unconnectedAccount": true
Expand Down
6 changes: 0 additions & 6 deletions test/e2e/default-fixture.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,6 @@ function defaultFixture(inputChainId = CHAIN_IDS.LOCALHOST) {
trezorModel: null,
newPrivacyPolicyToastClickedOrClosed: true,
newPrivacyPolicyToastShownDate: Date.now(),
usedNetworks: {
[CHAIN_IDS.MAINNET]: true,
[CHAIN_IDS.LINEA_MAINNET]: true,
[CHAIN_IDS.GOERLI]: true,
[CHAIN_IDS.LOCALHOST]: true,
},
snapsInstallPrivacyWarningShown: true,
},
BridgeController: {
Expand Down
6 changes: 0 additions & 6 deletions test/e2e/fixture-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,6 @@ function onboardingFixture() {
'__FIXTURE_SUBSTITUTION__currentDateInMilliseconds',
showTestnetMessageInDropdown: true,
trezorModel: null,
usedNetworks: {
[CHAIN_IDS.MAINNET]: true,
[CHAIN_IDS.LINEA_MAINNET]: true,
[CHAIN_IDS.GOERLI]: true,
[CHAIN_IDS.LOCALHOST]: true,
},
},
NetworkController: {
...mockNetworkStateOld({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
"nftsDropdownState": {},
"termsOfUseLastAgreed": "number",
"qrHardware": {},
"usedNetworks": { "0x1": true, "0x5": true, "0x539": true },
"snapsInstallPrivacyWarningShown": true,
"surveyLinkLastClickedOrClosed": "object",
"signatureSecurityAlertResponses": "object",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@
"nftsDropdownState": {},
"termsOfUseLastAgreed": "number",
"qrHardware": {},
"usedNetworks": { "0x1": true, "0x5": true, "0x539": true },
"snapsInstallPrivacyWarningShown": true,
"surveyLinkLastClickedOrClosed": "object",
"signatureSecurityAlertResponses": "object",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,6 @@
"trezorModel": null,
"newPrivacyPolicyToastClickedOrClosed": "boolean",
"newPrivacyPolicyToastShownDate": "number",
"usedNetworks": {
"0x1": true,
"0xe708": true,
"0x5": true,
"0x539": true
},
"snapsInstallPrivacyWarningShown": true
},
"CurrencyController": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,6 @@
"trezorModel": null,
"newPrivacyPolicyToastClickedOrClosed": "boolean",
"newPrivacyPolicyToastShownDate": "number",
"usedNetworks": {
"0x1": true,
"0xe708": true,
"0x5": true,
"0x539": true
},
"snapsInstallPrivacyWarningShown": true
},
"BridgeController": {
Expand Down
9 changes: 0 additions & 9 deletions test/integration/data/integration-init-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -2052,15 +2052,6 @@
"useSafeChainsListValidation": true,
"useTokenDetection": false,
"useTransactionSimulations": true,
"usedNetworks": {
"0xaa36a7": {
"rpcUrl": "https://sepolia.infura.io/v3/dummy_key",
"chainId": "0xaa36a7",
"nickname": "Sepolia Test Network",
"ticker": "ETH",
"blockExplorerUrl": "https://sepolia.etherscan.io"
}
},
"userOperations": {},
"versionFileETag": "W/\"598946a37f16c5b882a0bebbadf4509f\"",
"versionInfo": [],
Expand Down
1 change: 0 additions & 1 deletion test/integration/data/onboarding-completion-route.json
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,6 @@
"useSafeChainsListValidation": true,
"useTokenDetection": true,
"useTransactionSimulations": true,
"usedNetworks": { "0x1": true, "0x5": true, "0x539": true },
"userOperations": {},
"versionFileETag": "",
"versionInfo": [],
Expand Down
2 changes: 0 additions & 2 deletions ui/components/ui/new-network-info/new-network-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
getParticipateInMetaMetrics,
getDataCollectionForMarketing,
} from '../../../selectors';
import { setFirstTimeUsedNetwork } from '../../../store/actions';
import {
PickerNetwork,
Text,
Expand Down Expand Up @@ -56,7 +55,6 @@ export default function NewNetworkInfo() {

const onCloseClick = () => {
setShowPopup(false);
setFirstTimeUsedNetwork(providerConfig.chainId);
};

const checkTokenDetection = useCallback(async () => {
Expand Down
8 changes: 0 additions & 8 deletions ui/pages/routes/routes.component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,12 +225,6 @@ describe('Routes Component', () => {
},
selectedAccount: account.id,
},
usedNetworks: {
'0x1': true,
'0x5': true,
'0x539': true,
[mockNewlyAddedNetwork.chainId]: false,
},
networkConfigurationsByChainId: {
...mockState.metamask.networkConfigurationsByChainId,
[mockNewlyAddedNetwork.chainId]: mockNewlyAddedNetwork,
Expand Down Expand Up @@ -312,7 +306,6 @@ describe('toast display', () => {
announcements: {},
approvalFlows: [],
completedOnboarding: true,
usedNetworks: [],
pendingApprovals: {},
pendingApprovalCount: 0,
preferences: {
Expand Down Expand Up @@ -341,7 +334,6 @@ describe('toast display', () => {
announcements: {},
approvalFlows: [],
completedOnboarding: true,
usedNetworks: [],
pendingApprovals: {},
pendingApprovalCount: 0,
swapsState: { swapsFeatureIsLive: true },
Expand Down
2 changes: 0 additions & 2 deletions ui/pages/routes/routes.container.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
} from '../../../shared/modules/selectors/networks';
import {
getAllAccountsOnNetworkAreEmpty,
getIsNetworkUsed,
getNetworkIdentifier,
getPreferences,
getTheme,
Expand Down Expand Up @@ -95,7 +94,6 @@ function mapStateToProps(state) {
providerType: getProviderConfig(state).type,
theme: getTheme(state),
sendStage: getSendStage(state),
isNetworkUsed: getIsNetworkUsed(state),
allAccountsOnNetworkAreEmpty: getAllAccountsOnNetworkAreEmpty(state),
isTestNet: getIsTestnet(state),
showExtensionInFullSizeView: getShowExtensionInFullSizeView(state),
Expand Down
7 changes: 0 additions & 7 deletions ui/selectors/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2726,13 +2726,6 @@ export function getBlockExplorerLinkText(
return blockExplorerLinkText;
}

export function getIsNetworkUsed(state) {
const chainId = getCurrentChainId(state);
const { usedNetworks } = state.metamask;

return Boolean(usedNetworks[chainId]);
}

export function getAllAccountsOnNetworkAreEmpty(state) {
const balances = getMetaMaskCachedBalances(state) ?? {};
const hasNoNativeFundsOnAnyAccounts = Object.values(balances).every(
Expand Down
4 changes: 0 additions & 4 deletions ui/store/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5201,10 +5201,6 @@ export function setUseTransactionSimulations(val: boolean): void {
}
}

export function setFirstTimeUsedNetwork(chainId: string) {
return submitRequestToBackground('setFirstTimeUsedNetwork', [chainId]);
}

// QR Hardware Wallets
export async function submitQRHardwareCryptoHDKey(cbor: Hex) {
await submitRequestToBackground('submitQRHardwareCryptoHDKey', [cbor]);
Expand Down

0 comments on commit 7143c96

Please sign in to comment.