From 5e52a7d6d05db865fefdb7e7fdab82b090089911 Mon Sep 17 00:00:00 2001 From: chloeYue <105063779+chloeYue@users.noreply.github.com> Date: Fri, 29 Nov 2024 12:14:33 +0100 Subject: [PATCH 1/3] test: [POM] fix change language flaky tests and migrate tests to Page Object Model (#28777) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** - Fix change language flaky tests. There are multiple reasons for the flakiness, including taking actions during the loading spinner and excessive misuse of refresh page - Migrate change language e2e tests to Page Object Model - Created base pages for advanced settings page and general settings page - For some special language-related locators, as they are only used in this specific test, I didn't migrate them to POM methods. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27155?quickstart=1) ## **Related issues** Fixes:https://github.com/MetaMask/metamask-extension/issues/27904 https://github.com/MetaMask/metamask-extension/issues/28698 https://github.com/MetaMask/metamask-extension/issues/27390 ## **Manual testing steps** Check code readability, make sure tests pass. ## **Screenshots/Recordings** ### **Before** ### **After** ## **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** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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. --- test/e2e/page-objects/common.ts | 5 +- .../pages/settings/advanced-settings.ts | 32 ++++ .../pages/settings/general-settings.ts | 61 +++++++ .../tests/settings/change-language.spec.ts | 153 +++++++++--------- 4 files changed, 175 insertions(+), 76 deletions(-) create mode 100644 test/e2e/page-objects/pages/settings/advanced-settings.ts create mode 100644 test/e2e/page-objects/pages/settings/general-settings.ts diff --git a/test/e2e/page-objects/common.ts b/test/e2e/page-objects/common.ts index b1f678b80cca..5bf1a91e1859 100644 --- a/test/e2e/page-objects/common.ts +++ b/test/e2e/page-objects/common.ts @@ -1 +1,4 @@ -export type RawLocator = string | { css: string; text: string }; +export type RawLocator = + | string + | { css?: string; text?: string } + | { tag: string; text: string }; diff --git a/test/e2e/page-objects/pages/settings/advanced-settings.ts b/test/e2e/page-objects/pages/settings/advanced-settings.ts new file mode 100644 index 000000000000..b93f29b58736 --- /dev/null +++ b/test/e2e/page-objects/pages/settings/advanced-settings.ts @@ -0,0 +1,32 @@ +import { Driver } from '../../../webdriver/driver'; + +class AdvancedSettings { + private readonly driver: Driver; + + private readonly downloadDataButton = '[data-testid="export-data-button"]'; + + private readonly downloadStateLogsButton = + '[data-testid="advanced-setting-state-logs"]'; + + constructor(driver: Driver) { + this.driver = driver; + } + + async check_pageIsLoaded(): Promise { + try { + await this.driver.waitForMultipleSelectors([ + this.downloadStateLogsButton, + this.downloadDataButton, + ]); + } catch (e) { + console.log( + 'Timeout while waiting for Advanced Settings page to be loaded', + e, + ); + throw e; + } + console.log('Advanced Settings page is loaded'); + } +} + +export default AdvancedSettings; diff --git a/test/e2e/page-objects/pages/settings/general-settings.ts b/test/e2e/page-objects/pages/settings/general-settings.ts new file mode 100644 index 000000000000..0db61a03a23f --- /dev/null +++ b/test/e2e/page-objects/pages/settings/general-settings.ts @@ -0,0 +1,61 @@ +import { Driver } from '../../../webdriver/driver'; + +class GeneralSettings { + private readonly driver: Driver; + + private readonly generalSettingsPageTitle = { + text: 'General', + tag: 'h4', + }; + + private readonly loadingOverlaySpinner = '.loading-overlay__spinner'; + + private readonly selectLanguageField = '[data-testid="locale-select"]'; + + constructor(driver: Driver) { + this.driver = driver; + } + + async check_pageIsLoaded(): Promise { + try { + await this.check_noLoadingOverlaySpinner(); + await this.driver.waitForMultipleSelectors([ + this.generalSettingsPageTitle, + this.selectLanguageField, + ]); + } catch (e) { + console.log( + 'Timeout while waiting for General Settings page to be loaded', + e, + ); + throw e; + } + console.log('General Settings page is loaded'); + } + + /** + * Change the language of MM on General Settings page + * + * @param languageToSelect - The language to select + */ + async changeLanguage(languageToSelect: string): Promise { + console.log( + 'Changing language to ', + languageToSelect, + 'on general settings page', + ); + await this.check_noLoadingOverlaySpinner(); + await this.driver.clickElement(this.selectLanguageField); + await this.driver.clickElement({ + text: languageToSelect, + tag: 'option', + }); + await this.check_noLoadingOverlaySpinner(); + } + + async check_noLoadingOverlaySpinner(): Promise { + await this.driver.assertElementNotPresent(this.loadingOverlaySpinner); + } +} + +export default GeneralSettings; diff --git a/test/e2e/tests/settings/change-language.spec.ts b/test/e2e/tests/settings/change-language.spec.ts index 1bd9915a33da..469ad3ceec30 100644 --- a/test/e2e/tests/settings/change-language.spec.ts +++ b/test/e2e/tests/settings/change-language.spec.ts @@ -1,21 +1,17 @@ import { strict as assert } from 'assert'; import { Suite } from 'mocha'; - import { Driver } from '../../webdriver/driver'; -import { - defaultGanacheOptions, - withFixtures, - unlockWallet, -} from '../../helpers'; +import { withFixtures } from '../../helpers'; import FixtureBuilder from '../../fixture-builder'; +import AdvancedSettings from '../../page-objects/pages/settings/advanced-settings'; +import GeneralSettings from '../../page-objects/pages/settings/general-settings'; +import HeaderNavbar from '../../page-objects/pages/header-navbar'; +import Homepage from '../../page-objects/pages/homepage'; +import SendTokenPage from '../../page-objects/pages/send/send-token-page'; +import SettingsPage from '../../page-objects/pages/settings/settings-page'; +import { loginWithBalanceValidation } from '../../page-objects/flows/login.flow'; const selectors = { - accountOptionsMenuButton: '[data-testid="account-options-menu-button"]', - settingsOption: { text: 'Settings', tag: 'div' }, - localeSelect: '[data-testid="locale-select"]', - ethOverviewSend: '[data-testid="eth-overview-send"]', - ensInput: '[data-testid="ens-input"]', - nftsTab: '[data-testid="account-overview__nfts-tab"]', labelSpanish: { tag: 'p', text: 'Idioma actual' }, currentLanguageLabel: { tag: 'p', text: 'Current language' }, advanceText: { text: 'Avanceret', tag: 'div' }, @@ -29,50 +25,37 @@ const selectors = { headerText: { text: 'الإعدادات', tag: 'h3' }, }; -async function changeLanguage(driver: Driver, languageIndex: number) { - await driver.clickElement(selectors.accountOptionsMenuButton); - await driver.clickElement(selectors.settingsOption); - - const dropdownElement = await driver.findElement(selectors.localeSelect); - await dropdownElement.click(); - - const options = await dropdownElement.findElements({ css: 'option' }); - await options[languageIndex].click(); -} - describe('Settings - general tab @no-mmi', function (this: Suite) { it('validate the change language functionality', async function () { - let languageIndex = 10; - await withFixtures( { fixtures: new FixtureBuilder().build(), - ganacheOptions: defaultGanacheOptions, title: this.test?.fullTitle(), }, - async ({ driver }: { driver: Driver }) => { - await unlockWallet(driver); - await changeLanguage(driver, languageIndex); + await loginWithBalanceValidation(driver); + await new HeaderNavbar(driver).openSettingsPage(); + const generalSettings = new GeneralSettings(driver); + await generalSettings.check_pageIsLoaded(); - // Validate the label changes to Spanish + // Change language to Spanish and validate that the word has changed correctly + await generalSettings.changeLanguage('Español'); const isLanguageLabelChanged = await driver.isElementPresent( selectors.labelSpanish, ); assert.equal(isLanguageLabelChanged, true, 'Language did not change'); + // Refresh the page and validate that the language is still Spanish await driver.refresh(); - - // Change back to English and verify that the word is correctly changed back to English - languageIndex = 9; - - const dropdownElement = await driver.findElement( - selectors.localeSelect, + await generalSettings.check_pageIsLoaded(); + assert.equal( + await driver.isElementPresent(selectors.labelSpanish), + true, + 'Language did not change after refresh', ); - await dropdownElement.click(); - const options = await dropdownElement.findElements({ css: 'option' }); - await options[languageIndex].click(); + // Change language back to English and validate that the word has changed correctly + await generalSettings.changeLanguage('English'); const isLabelTextChanged = await driver.isElementPresent( selectors.currentLanguageLabel, ); @@ -82,21 +65,22 @@ describe('Settings - general tab @no-mmi', function (this: Suite) { }); it('validate "Dansk" language on page navigation', async function () { - const languageIndex = 6; await withFixtures( { fixtures: new FixtureBuilder().build(), - ganacheOptions: defaultGanacheOptions, title: this.test?.fullTitle(), }, - async ({ driver }: { driver: Driver }) => { - await unlockWallet(driver); - await changeLanguage(driver, languageIndex); - - await driver.assertElementNotPresent('.loading-overlay__spinner'); + await loginWithBalanceValidation(driver); + await new HeaderNavbar(driver).openSettingsPage(); + const generalSettings = new GeneralSettings(driver); + await generalSettings.check_pageIsLoaded(); + // Select "Dansk" language + await generalSettings.changeLanguage('Dansk'); await driver.clickElement(selectors.advanceText); + const advancedSettings = new AdvancedSettings(driver); + await advancedSettings.check_pageIsLoaded(); // Confirm that the language change is reflected in search box water text const isWaterTextChanged = await driver.isElementPresent( @@ -132,22 +116,30 @@ describe('Settings - general tab @no-mmi', function (this: Suite) { }); it('validate "Deutsch" language on error messages', async function () { - const languageIndex = 7; await withFixtures( { fixtures: new FixtureBuilder().build(), - ganacheOptions: defaultGanacheOptions, title: this.test?.fullTitle(), }, - async ({ driver }: { driver: Driver }) => { - await unlockWallet(driver); - await changeLanguage(driver, languageIndex); - await driver.navigate(); - await driver.clickElement(selectors.ethOverviewSend); - await driver.pasteIntoField( - selectors.ensInput, - // use wrong checksum address; other inputs don't show error until snaps name-lookup has happened + await loginWithBalanceValidation(driver); + await new HeaderNavbar(driver).openSettingsPage(); + const generalSettings = new GeneralSettings(driver); + await generalSettings.check_pageIsLoaded(); + + // Select "Deutsch" language + await generalSettings.changeLanguage('Deutsch'); + await new SettingsPage(driver).closeSettingsPage(); + + const homepage = new Homepage(driver); + await homepage.check_pageIsLoaded(); + await homepage.check_expectedBalanceIsDisplayed(); + await homepage.startSendFlow(); + + const sendToPage = new SendTokenPage(driver); + await sendToPage.check_pageIsLoaded(); + // use wrong address for recipient to allow error message to show + await sendToPage.fillRecipient( '0xAAAA6BF26964aF9D7eEd9e03E53415D37aA96045', ); @@ -165,18 +157,23 @@ describe('Settings - general tab @no-mmi', function (this: Suite) { }); it('validate "मानक हिन्दी" language on tooltips', async function () { - const languageIndex = 19; await withFixtures( { fixtures: new FixtureBuilder().build(), - ganacheOptions: defaultGanacheOptions, title: this.test?.fullTitle(), }, - async ({ driver }: { driver: Driver }) => { - await unlockWallet(driver); - await changeLanguage(driver, languageIndex); - await driver.navigate(); + await loginWithBalanceValidation(driver); + await new HeaderNavbar(driver).openSettingsPage(); + const generalSettings = new GeneralSettings(driver); + await generalSettings.check_pageIsLoaded(); + + // Select "मानक हिन्दी" language + await generalSettings.changeLanguage('मानक हिन्दी'); + await new SettingsPage(driver).closeSettingsPage(); + const homepage = new Homepage(driver); + await homepage.check_pageIsLoaded(); + await homepage.check_expectedBalanceIsDisplayed(); // Validate the account tooltip const isAccountTooltipChanged = await driver.isElementPresent( @@ -202,20 +199,24 @@ describe('Settings - general tab @no-mmi', function (this: Suite) { }); it('validate "Magyar" language change on hypertext', async function () { - const languageIndex = 23; await withFixtures( { fixtures: new FixtureBuilder().build(), - ganacheOptions: defaultGanacheOptions, title: this.test?.fullTitle(), }, - async ({ driver }: { driver: Driver }) => { - await unlockWallet(driver); - // selects "Magyar" language - await changeLanguage(driver, languageIndex); - await driver.navigate(); - await driver.clickElement(selectors.nftsTab); + await loginWithBalanceValidation(driver); + await new HeaderNavbar(driver).openSettingsPage(); + const generalSettings = new GeneralSettings(driver); + await generalSettings.check_pageIsLoaded(); + + // Select "Magyar" language + await generalSettings.changeLanguage('Magyar'); + await new SettingsPage(driver).closeSettingsPage(); + const homepage = new Homepage(driver); + await homepage.check_pageIsLoaded(); + await homepage.check_expectedBalanceIsDisplayed(); + await homepage.goToNftTab(); // Validate the hypertext const isHyperTextChanged = await driver.isElementPresent( @@ -231,18 +232,20 @@ describe('Settings - general tab @no-mmi', function (this: Suite) { }); it('validate "العربية" language change on page indent', async function () { - const languageIndex = 1; await withFixtures( { fixtures: new FixtureBuilder().build(), - ganacheOptions: defaultGanacheOptions, title: this.test?.fullTitle(), }, async ({ driver }: { driver: Driver }) => { - await unlockWallet(driver); - await changeLanguage(driver, languageIndex); + await loginWithBalanceValidation(driver); + await new HeaderNavbar(driver).openSettingsPage(); + const generalSettings = new GeneralSettings(driver); + await generalSettings.check_pageIsLoaded(); + + // Select "العربية" language and validate that the header text has changed + await generalSettings.changeLanguage('العربية'); - // Validate the header text const isHeaderTextChanged = await driver.isElementPresent( selectors.headerText, ); From 7d252e9ca78e40c7e41034667850f07da8c70ca0 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 29 Nov 2024 16:25:29 +0000 Subject: [PATCH 2/3] refactor: remove global network from transaction controller (#28449) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Upgrade `@metamask/transaction-controller` to remove all usages of the global network. Specifically: - Remove deleted constructor options. - Add `getGlobalChainId` and `getGlobalNetworkClientId` private methods in `MetamaskController`. - Remove `TRANSACTION_MULTICHAIN` environment variable. - Add `networkClientId` to test data. - Update calls to: - `addTransaction` - `estimateGasBuffered` - `getNonceLock` - `getTransactions` - `startIncomingTransactionPolling` - `updateIncomingTransactions` - `wipeTransactions` [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28449?quickstart=1) ## **Related issues** Fixes: [#3499](https://github.com/MetaMask/MetaMask-planning/issues/3499) ## **Manual testing steps** Full regression of all transaction flows. ## **Screenshots/Recordings** ### **Before** ### **After** ## **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. --- app/scripts/controllers/mmi-controller.ts | 2 +- app/scripts/lib/transaction/metrics.test.ts | 1 + .../transaction/smart-transactions.test.ts | 1 + app/scripts/lib/transaction/util.test.ts | 39 +---- app/scripts/lib/transaction/util.ts | 7 +- app/scripts/metamask-controller.js | 135 +++++++++++------- app/scripts/metamask-controller.test.js | 10 +- builds.yml | 2 - package.json | 2 +- ui/ducks/swaps/swaps.js | 1 + .../useFirstTimeInteractionAlert.test.ts | 1 + .../usePendingTransactionAlerts.test.ts | 1 + .../transactions/useResimulationAlert.test.ts | 1 + .../useSigningOrSubmittingAlerts.test.ts | 1 + ui/store/actions.ts | 1 - yarn.lock | 10 +- 16 files changed, 110 insertions(+), 105 deletions(-) diff --git a/app/scripts/controllers/mmi-controller.ts b/app/scripts/controllers/mmi-controller.ts index 9f2701b2c0b9..f94388aa5e8f 100644 --- a/app/scripts/controllers/mmi-controller.ts +++ b/app/scripts/controllers/mmi-controller.ts @@ -311,7 +311,7 @@ export class MMIController { } } - const txList = this.txStateManager.getTransactions({}, [], false); // Includes all transactions, but we are looping through keyrings. Currently filtering is done in updateCustodianTransactions :-/ + const txList = this.txStateManager.getTransactions(); // Includes all transactions, but we are looping through keyrings. Currently filtering is done in updateCustodianTransactions :-/ try { updateCustodianTransactions({ diff --git a/app/scripts/lib/transaction/metrics.test.ts b/app/scripts/lib/transaction/metrics.test.ts index 7dcedd4e467e..c4ddf9d29e5a 100644 --- a/app/scripts/lib/transaction/metrics.test.ts +++ b/app/scripts/lib/transaction/metrics.test.ts @@ -115,6 +115,7 @@ describe('Transaction metrics', () => { type: TransactionType.simpleSend, origin: ORIGIN_METAMASK, chainId: mockChainId, + networkClientId: 'testNetworkClientId', time: 1624408066355, defaultGasEstimates: { gas: '0x7b0d', diff --git a/app/scripts/lib/transaction/smart-transactions.test.ts b/app/scripts/lib/transaction/smart-transactions.test.ts index 1cf266c3e141..25bb409dffa1 100644 --- a/app/scripts/lib/transaction/smart-transactions.test.ts +++ b/app/scripts/lib/transaction/smart-transactions.test.ts @@ -150,6 +150,7 @@ function withRequest( }, type: TransactionType.simpleSend, chainId: CHAIN_IDS.MAINNET, + networkClientId: 'testNetworkClientId', time: 1624408066355, defaultGasEstimates: { gas: '0x7b0d', diff --git a/app/scripts/lib/transaction/util.test.ts b/app/scripts/lib/transaction/util.test.ts index fbbee025381b..4d78ea51cfa5 100644 --- a/app/scripts/lib/transaction/util.test.ts +++ b/app/scripts/lib/transaction/util.test.ts @@ -50,6 +50,7 @@ const TRANSACTION_PARAMS_MOCK: TransactionParams = { const TRANSACTION_OPTIONS_MOCK: AddTransactionOptions = { actionId: 'mockActionId', + networkClientId: 'mockNetworkClientId', origin: 'mockOrigin', requireApproval: false, type: TransactionType.simpleSend, @@ -151,23 +152,6 @@ describe('Transaction Utils', () => { }); }); - it('adds transaction with networkClientId if process.env.TRANSACTION_MULTICHAIN is set', async () => { - process.env.TRANSACTION_MULTICHAIN = '1'; - - await addTransaction(request); - - expect( - request.transactionController.addTransaction, - ).toHaveBeenCalledTimes(1); - expect( - request.transactionController.addTransaction, - ).toHaveBeenCalledWith(TRANSACTION_PARAMS_MOCK, { - ...TRANSACTION_OPTIONS_MOCK, - networkClientId: 'mockNetworkClientId', - }); - process.env.TRANSACTION_MULTICHAIN = ''; - }); - it('returns transaction meta', async () => { const transactionMeta = await addTransaction(request); expect(transactionMeta).toStrictEqual(TRANSACTION_META_MOCK); @@ -541,27 +525,6 @@ describe('Transaction Utils', () => { }); }); - it('adds transaction with networkClientId if process.env.TRANSACTION_MULTICHAIN is set', async () => { - process.env.TRANSACTION_MULTICHAIN = '1'; - - await addDappTransaction(dappRequest); - - expect( - request.transactionController.addTransaction, - ).toHaveBeenCalledTimes(1); - expect( - request.transactionController.addTransaction, - ).toHaveBeenCalledWith(TRANSACTION_PARAMS_MOCK, { - ...TRANSACTION_OPTIONS_MOCK, - networkClientId: 'mockNetworkClientId', - method: DAPP_REQUEST_MOCK.method, - requireApproval: true, - securityAlertResponse: DAPP_REQUEST_MOCK.securityAlertResponse, - type: undefined, - }); - process.env.TRANSACTION_MULTICHAIN = ''; - }); - it('returns transaction hash', async () => { const transactionHash = await addDappTransaction(dappRequest); expect(transactionHash).toStrictEqual(TRANSACTION_META_MOCK.hash); diff --git a/app/scripts/lib/transaction/util.ts b/app/scripts/lib/transaction/util.ts index 0bbf93afd8a8..34f27d321e0b 100644 --- a/app/scripts/lib/transaction/util.ts +++ b/app/scripts/lib/transaction/util.ts @@ -46,7 +46,7 @@ type BaseAddTransactionRequest = { }; type FinalAddTransactionRequest = BaseAddTransactionRequest & { - transactionOptions: AddTransactionOptions; + transactionOptions: Partial; }; export type AddTransactionRequest = FinalAddTransactionRequest & { @@ -66,7 +66,7 @@ export async function addDappTransaction( const { id: actionId, method, origin } = dappRequest; const { securityAlertResponse, traceContext } = dappRequest; - const transactionOptions: AddTransactionOptions = { + const transactionOptions: Partial = { actionId, method, origin, @@ -143,10 +143,11 @@ async function addTransactionWithController( transactionParams, networkClientId, } = request; + const { result, transactionMeta } = await transactionController.addTransaction(transactionParams, { ...transactionOptions, - ...(process.env.TRANSACTION_MULTICHAIN ? { networkClientId } : {}), + networkClientId, }); return { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index d60d937e1c3c..a1cbabb9bcbe 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -236,10 +236,7 @@ import { TOKEN_TRANSFER_LOG_TOPIC_HASH, TRANSFER_SINFLE_LOG_TOPIC_HASH, } from '../../shared/lib/transactions-controller-utils'; -import { - getCurrentChainId, - getProviderConfig, -} from '../../shared/modules/selectors/networks'; +import { getProviderConfig } from '../../shared/modules/selectors/networks'; import { endTrace, trace } from '../../shared/lib/trace'; // eslint-disable-next-line import/no-restricted-paths import { isSnapId } from '../../ui/helpers/utils/snaps'; @@ -652,7 +649,9 @@ export default class MetamaskController extends EventEmitter { }); this.tokenListController = new TokenListController({ - chainId: getCurrentChainId({ metamask: this.networkController.state }), + chainId: this.#getGlobalChainId({ + metamask: this.networkController.state, + }), preventPollingOnNetworkRestart: !this.#isTokenListPollingRequired( this.preferencesController.state, ), @@ -676,7 +675,7 @@ export default class MetamaskController extends EventEmitter { }); this.assetsContractController = new AssetsContractController({ messenger: assetsContractControllerMessenger, - chainId: getCurrentChainId({ metamask: this.networkController.state }), + chainId: this.#getGlobalChainId(), }); const tokensControllerMessenger = this.controllerMessenger.getRestricted({ @@ -699,7 +698,7 @@ export default class MetamaskController extends EventEmitter { state: initState.TokensController, provider: this.provider, messenger: tokensControllerMessenger, - chainId: getCurrentChainId({ metamask: this.networkController.state }), + chainId: this.#getGlobalChainId(), }); const nftControllerMessenger = this.controllerMessenger.getRestricted({ @@ -725,7 +724,7 @@ export default class MetamaskController extends EventEmitter { this.nftController = new NftController({ state: initState.NftController, messenger: nftControllerMessenger, - chainId: getCurrentChainId({ metamask: this.networkController.state }), + chainId: this.#getGlobalChainId(), onNftAdded: ({ address, symbol, tokenId, standard, source }) => this.metaMetricsController.trackEvent({ event: MetaMetricsEventName.NftAdded, @@ -760,7 +759,7 @@ export default class MetamaskController extends EventEmitter { this.nftDetectionController = new NftDetectionController({ messenger: nftDetectionControllerMessenger, - chainId: getCurrentChainId({ metamask: this.networkController.state }), + chainId: this.#getGlobalChainId(), getOpenSeaApiKey: () => this.nftController.openSeaApiKey, getBalancesInSingleCall: this.assetsContractController.getBalancesInSingleCall.bind( @@ -849,13 +848,10 @@ export default class MetamaskController extends EventEmitter { legacyAPIEndpoint: `${gasApiBaseUrl}/networks//gasPrices`, EIP1559APIEndpoint: `${gasApiBaseUrl}/networks//suggestedGasFees`, getCurrentNetworkLegacyGasAPICompatibility: () => { - const chainId = getCurrentChainId({ - metamask: this.networkController.state, - }); + const chainId = this.#getGlobalChainId(); return chainId === CHAIN_IDS.BSC; }, - getChainId: () => - getCurrentChainId({ metamask: this.networkController.state }), + getChainId: () => this.#getGlobalChainId(), }); this.appStateController = new AppStateController({ @@ -950,7 +946,7 @@ export default class MetamaskController extends EventEmitter { ppomInit: () => PPOMModule.default(process.env.PPOM_URI), }, state: initState.PPOMController, - chainId: getCurrentChainId({ metamask: this.networkController.state }), + chainId: this.#getGlobalChainId(), securityAlertsEnabled: this.preferencesController.state.securityAlertsEnabled, onPreferencesChange: preferencesMessenger.subscribe.bind( @@ -1916,8 +1912,8 @@ export default class MetamaskController extends EventEmitter { ], allowedEvents: [`NetworkController:stateChange`], }); + this.txController = new TransactionController({ - blockTracker: this.blockTracker, getCurrentNetworkEIP1559Compatibility: this.networkController.getEIP1559Compatibility.bind( this.networkController, @@ -1935,10 +1931,10 @@ export default class MetamaskController extends EventEmitter { ), getNetworkState: () => this.networkController.state, getPermittedAccounts: this.getPermittedAccounts.bind(this), - getSavedGasFees: () => - this.preferencesController.state.advancedGasFee[ - getCurrentChainId({ metamask: this.networkController.state }) - ], + getSavedGasFees: () => { + const globalChainId = this.#getGlobalChainId(); + return this.preferencesController.state.advancedGasFee[globalChainId]; + }, incomingTransactions: { etherscanApiKeysByChainId: { [CHAIN_IDS.MAINNET]: process.env.ETHERSCAN_API_KEY, @@ -1946,26 +1942,17 @@ export default class MetamaskController extends EventEmitter { }, includeTokenTransfers: false, isEnabled: () => - Boolean( - this.preferencesController.state.incomingTransactionsPreferences?.[ - getCurrentChainId({ metamask: this.networkController.state }) - ] && this.onboardingController.state.completedOnboarding, - ), + this.preferencesController.state.incomingTransactionsPreferences?.[ + this.#getGlobalChainId() + ] && this.onboardingController.state.completedOnboarding, queryEntireHistory: false, updateTransactions: false, }, isFirstTimeInteractionEnabled: () => this.preferencesController.state.securityAlertsEnabled, - isMultichainEnabled: process.env.TRANSACTION_MULTICHAIN, isSimulationEnabled: () => this.preferencesController.state.useTransactionSimulations, messenger: transactionControllerMessenger, - onNetworkStateChange: (listener) => { - networkControllerMessenger.subscribe( - 'NetworkController:networkDidChange', - () => listener(), - ); - }, pendingTransactions: { isResubmitEnabled: () => { const state = this._getMetaMaskState(); @@ -1975,7 +1962,6 @@ export default class MetamaskController extends EventEmitter { ); }, }, - provider: this.provider, testGasFeeFlows: process.env.TEST_GAS_FEE_FLOWS, trace, hooks: { @@ -2155,12 +2141,12 @@ export default class MetamaskController extends EventEmitter { this.swapsController = new SwapsController( { messenger: swapsControllerMessenger, - // TODO: Remove once TransactionController exports this action type getBufferedGasLimit: async (txMeta, multiplier) => { const { gas: gasLimit, simulationFails } = await this.txController.estimateGasBuffered( txMeta.txParams, multiplier, + this.#getGlobalNetworkClientId(), ); return { gasLimit, simulationFails }; @@ -2223,7 +2209,11 @@ export default class MetamaskController extends EventEmitter { this.smartTransactionsController = new SmartTransactionsController({ supportedChainIds: getAllowedSmartTransactionsChainIds(), clientId: ClientId.Extension, - getNonceLock: this.txController.getNonceLock.bind(this.txController), + getNonceLock: (address) => + this.txController.getNonceLock( + address, + this.#getGlobalNetworkClientId(), + ), confirmExternalTransaction: this.txController.confirmExternalTransaction.bind(this.txController), trackMetaMetricsEvent: this.metaMetricsController.trackEvent.bind( @@ -2704,7 +2694,10 @@ export default class MetamaskController extends EventEmitter { } triggerNetworkrequests() { - this.txController.startIncomingTransactionPolling(); + this.txController.startIncomingTransactionPolling([ + this.#getGlobalNetworkClientId(), + ]); + this.tokenDetectionController.enable(); this.getInfuraFeatureFlags(); } @@ -2942,13 +2935,13 @@ export default class MetamaskController extends EventEmitter { 'PreferencesController:stateChange', previousValueComparator(async (prevState, currState) => { const { currentLocale } = currState; - const chainId = getCurrentChainId({ - metamask: this.networkController.state, - }); + const chainId = this.#getGlobalChainId(); await updateCurrentLocale(currentLocale); if (currState.incomingTransactionsPreferences?.[chainId]) { - this.txController.startIncomingTransactionPolling(); + this.txController.startIncomingTransactionPolling([ + this.#getGlobalNetworkClientId(), + ]); } else { this.txController.stopIncomingTransactionPolling(); } @@ -3018,7 +3011,15 @@ export default class MetamaskController extends EventEmitter { this.controllerMessenger.subscribe( 'NetworkController:networkDidChange', async () => { - await this.txController.updateIncomingTransactions(); + await this.txController.updateIncomingTransactions([ + this.#getGlobalNetworkClientId(), + ]); + + await this.txController.stopIncomingTransactionPolling(); + + await this.txController.startIncomingTransactionPolling([ + this.#getGlobalNetworkClientId(), + ]); }, ); @@ -4520,9 +4521,7 @@ export default class MetamaskController extends EventEmitter { async _addAccountsWithBalance() { try { // Scan accounts until we find an empty one - const chainId = getCurrentChainId({ - metamask: this.networkController.state, - }); + const chainId = this.#getGlobalChainId(); const ethQuery = new EthQuery(this.provider); const accounts = await this.keyringController.getAccounts(); let address = accounts[accounts.length - 1]; @@ -5051,15 +5050,24 @@ export default class MetamaskController extends EventEmitter { async resetAccount() { const selectedAddress = this.accountsController.getSelectedAccount().address; - this.txController.wipeTransactions(false, selectedAddress); + + const globalChainId = this.#getGlobalChainId(); + + this.txController.wipeTransactions({ + address: selectedAddress, + chainId: globalChainId, + }); + this.smartTransactionsController.wipeSmartTransactions({ address: selectedAddress, ignoreNetwork: false, }); + this.bridgeStatusController.wipeBridgeStatus({ address: selectedAddress, ignoreNetwork: false, }); + this.networkController.resetConnection(); return selectedAddress; @@ -5187,8 +5195,7 @@ export default class MetamaskController extends EventEmitter { internalAccounts: this.accountsController.listAccounts(), dappRequest, networkClientId: - dappRequest?.networkClientId ?? - this.networkController.state.selectedNetworkClientId, + dappRequest?.networkClientId ?? this.#getGlobalNetworkClientId(), selectedAccount: this.accountsController.getAccountByAddress( transactionParams.from, ), @@ -5196,7 +5203,7 @@ export default class MetamaskController extends EventEmitter { transactionOptions, transactionParams, userOperationController: this.userOperationController, - chainId: getCurrentChainId({ metamask: this.networkController.state }), + chainId: this.#getGlobalChainId(), ppomController: this.ppomController, securityAlertsEnabled: this.preferencesController.state?.securityAlertsEnabled, @@ -6546,13 +6553,13 @@ export default class MetamaskController extends EventEmitter { * Returns the nonce that will be associated with a transaction once approved * * @param {string} address - The hex string address for the transaction - * @param networkClientId - The optional networkClientId to get the nonce lock with + * @param networkClientId - The networkClientId to get the nonce lock with * @returns {Promise} */ async getPendingNonce(address, networkClientId) { const { nonceDetails, releaseLock } = await this.txController.getNonceLock( address, - process.env.TRANSACTION_MULTICHAIN ? networkClientId : undefined, + networkClientId, ); const pendingNonce = nonceDetails.params.highestSuggested; @@ -6565,13 +6572,13 @@ export default class MetamaskController extends EventEmitter { * Returns the next nonce according to the nonce-tracker * * @param {string} address - The hex string address for the transaction - * @param networkClientId - The optional networkClientId to get the nonce lock with + * @param networkClientId - The networkClientId to get the nonce lock with * @returns {Promise} */ async getNextNonce(address, networkClientId) { const nonceLock = await this.txController.getNonceLock( address, - process.env.TRANSACTION_MULTICHAIN ? networkClientId : undefined, + networkClientId, ); nonceLock.releaseLock(); return nonceLock.nextNonce; @@ -7378,4 +7385,28 @@ export default class MetamaskController extends EventEmitter { return useTokenDetection || petnamesEnabled || useTransactionSimulations; } + + /** + * @deprecated Avoid new references to the global network. + * Will be removed once multi-chain support is fully implemented. + * @returns {string} The chain ID of the currently selected network. + */ + #getGlobalChainId() { + const globalNetworkClientId = this.#getGlobalNetworkClientId(); + + const globalNetworkClient = this.networkController.getNetworkClientById( + globalNetworkClientId, + ); + + return globalNetworkClient.configuration.chainId; + } + + /** + * @deprecated Avoid new references to the global network. + * Will be removed once multi-chain support is fully implemented. + * @returns {string} The network client ID of the currently selected network client. + */ + #getGlobalNetworkClientId() { + return this.networkController.state.selectedNetworkClientId; + } } diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index a596277154eb..f7115f9364bd 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -21,7 +21,10 @@ import { } from '@metamask/keyring-api'; import { ControllerMessenger } from '@metamask/base-controller'; import { LoggingController, LogType } from '@metamask/logging-controller'; -import { TransactionController } from '@metamask/transaction-controller'; +import { + CHAIN_IDS, + TransactionController, +} from '@metamask/transaction-controller'; import { RatesController, TokenListController, @@ -1206,7 +1209,10 @@ describe('MetaMaskController', () => { ).toHaveBeenCalledTimes(1); expect( metamaskController.txController.wipeTransactions, - ).toHaveBeenCalledWith(false, selectedAddressMock); + ).toHaveBeenCalledWith({ + address: selectedAddressMock, + chainId: CHAIN_IDS.MAINNET, + }); expect( metamaskController.smartTransactionsController.wipeSmartTransactions, ).toHaveBeenCalledWith({ diff --git a/builds.yml b/builds.yml index 3963aeda93e5..7985f464b73f 100644 --- a/builds.yml +++ b/builds.yml @@ -276,8 +276,6 @@ env: - NODE_DEBUG: '' # Used by react-devtools-core - EDITOR_URL: '' - # Determines if feature flagged Multichain Transactions should be used - - TRANSACTION_MULTICHAIN: '' # Determines if Barad Dur features should be used - BARAD_DUR: '' # Determines if feature flagged Chain permissions diff --git a/package.json b/package.json index e7827cb6d92b..803853059703 100644 --- a/package.json +++ b/package.json @@ -347,7 +347,7 @@ "@metamask/snaps-sdk": "^6.12.0", "@metamask/snaps-utils": "^8.6.0", "@metamask/solana-wallet-snap": "^0.1.9", - "@metamask/transaction-controller": "^40.1.0", + "@metamask/transaction-controller": "^41.0.0", "@metamask/user-operation-controller": "^16.0.0", "@metamask/utils": "^10.0.1", "@ngraveio/bc-ur": "^1.1.12", diff --git a/ui/ducks/swaps/swaps.js b/ui/ducks/swaps/swaps.js index bc1c675e4d21..e044fdf42b3f 100644 --- a/ui/ducks/swaps/swaps.js +++ b/ui/ducks/swaps/swaps.js @@ -589,6 +589,7 @@ export const fetchSwapsLivenessAndFeatureFlags = () => { await dispatch(fetchSmartTransactionsLiveness()); const transactions = await getTransactions({ searchCriteria: { + chainId, from: getSelectedInternalAccount(state)?.address, }, }); diff --git a/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.test.ts b/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.test.ts index 964b218e8501..6689d6610248 100644 --- a/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.test.ts +++ b/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.test.ts @@ -22,6 +22,7 @@ const CONFIRMATION_MOCK = genUnapprovedContractInteractionConfirmation({ const TRANSACTION_META_MOCK = { id: TRANSACTION_ID_MOCK, chainId: '0x5', + networkClientId: 'testNetworkClientId', status: TransactionStatus.submitted, type: TransactionType.contractInteraction, txParams: { diff --git a/ui/pages/confirmations/hooks/alerts/transactions/usePendingTransactionAlerts.test.ts b/ui/pages/confirmations/hooks/alerts/transactions/usePendingTransactionAlerts.test.ts index 5d2c16a518e5..b9b3d12dc7d0 100644 --- a/ui/pages/confirmations/hooks/alerts/transactions/usePendingTransactionAlerts.test.ts +++ b/ui/pages/confirmations/hooks/alerts/transactions/usePendingTransactionAlerts.test.ts @@ -22,6 +22,7 @@ const CONFIRMATION_MOCK = genUnapprovedContractInteractionConfirmation({ const TRANSACTION_META_MOCK = { id: TRANSACTION_ID_MOCK, chainId: '0x5', + networkClientId: 'testNetworkClientId', status: TransactionStatus.submitted, type: TransactionType.contractInteraction, txParams: { diff --git a/ui/pages/confirmations/hooks/alerts/transactions/useResimulationAlert.test.ts b/ui/pages/confirmations/hooks/alerts/transactions/useResimulationAlert.test.ts index 174b6ec0398a..49209f69df9c 100644 --- a/ui/pages/confirmations/hooks/alerts/transactions/useResimulationAlert.test.ts +++ b/ui/pages/confirmations/hooks/alerts/transactions/useResimulationAlert.test.ts @@ -22,6 +22,7 @@ const CONFIRMATION_MOCK = genUnapprovedContractInteractionConfirmation({ const TRANSACTION_META_MOCK = { id: TRANSACTION_ID_MOCK, chainId: '0x5', + networkClientId: 'testNetworkClientId', status: TransactionStatus.submitted, type: TransactionType.contractInteraction, txParams: { diff --git a/ui/pages/confirmations/hooks/alerts/transactions/useSigningOrSubmittingAlerts.test.ts b/ui/pages/confirmations/hooks/alerts/transactions/useSigningOrSubmittingAlerts.test.ts index d1960827c0b1..08ed8f2aac7d 100644 --- a/ui/pages/confirmations/hooks/alerts/transactions/useSigningOrSubmittingAlerts.test.ts +++ b/ui/pages/confirmations/hooks/alerts/transactions/useSigningOrSubmittingAlerts.test.ts @@ -28,6 +28,7 @@ const CONFIRMATION_MOCK = genUnapprovedContractInteractionConfirmation({ const TRANSACTION_META_MOCK = { id: TRANSACTION_ID_MOCK, chainId: '0x5', + networkClientId: 'testNetworkClientId', status: TransactionStatus.submitted, type: TransactionType.contractInteraction, txParams: { diff --git a/ui/store/actions.ts b/ui/store/actions.ts index 2ad3046e9040..9b402f476a7e 100644 --- a/ui/store/actions.ts +++ b/ui/store/actions.ts @@ -1129,7 +1129,6 @@ export function updateAndApproveTx( export async function getTransactions( filters: { - filterToCurrentNetwork?: boolean; searchCriteria?: Partial & Partial; } = {}, ): Promise { diff --git a/yarn.lock b/yarn.lock index 91cc4445c553..c88156929c17 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6431,9 +6431,9 @@ __metadata: languageName: node linkType: hard -"@metamask/transaction-controller@npm:^40.1.0": - version: 40.1.0 - resolution: "@metamask/transaction-controller@npm:40.1.0" +"@metamask/transaction-controller@npm:^41.0.0": + version: 41.0.0 + resolution: "@metamask/transaction-controller@npm:41.0.0" dependencies: "@ethereumjs/common": "npm:^3.2.0" "@ethereumjs/tx": "npm:^4.2.0" @@ -6460,7 +6460,7 @@ __metadata: "@metamask/approval-controller": ^7.0.0 "@metamask/gas-fee-controller": ^22.0.0 "@metamask/network-controller": ^22.0.0 - checksum: 10/1057af5b0da2d51e46e7568fc0e7fdbe6aed34a013cf56a5a35ad694cbedcb726a5823bbe70b980d1dc9560138acf9d82ac5f0e06f7d17e11b46abacd466dc42 + checksum: 10/67a00b2eade35fc4e635a6bcbbcd847b3986b3bdcc9730ff2c8f81234df18ed11149203c13d6bad616e859f7e25879efab36b6dc4be05a4e747b4280ae2f300d languageName: node linkType: hard @@ -26566,7 +26566,7 @@ __metadata: "@metamask/solana-wallet-snap": "npm:^0.1.9" "@metamask/test-bundler": "npm:^1.0.0" "@metamask/test-dapp": "npm:8.13.0" - "@metamask/transaction-controller": "npm:^40.1.0" + "@metamask/transaction-controller": "npm:^41.0.0" "@metamask/user-operation-controller": "npm:^16.0.0" "@metamask/utils": "npm:^10.0.1" "@ngraveio/bc-ur": "npm:^1.1.12" From 7143c9643a11034a2a8b7f5f644f961be71b5e10 Mon Sep 17 00:00:00 2001 From: cryptodev-2s <109512101+cryptodev-2s@users.noreply.github.com> Date: Fri, 29 Nov 2024 18:03:37 +0100 Subject: [PATCH 3/3] chore: remove unused `usedNetworks` state property from `AppStateController` (#28813) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Building on the work done to remove the network modal in [PR](https://github.com/MetaMask/metamask-extension/pull/28765), this PR finalizes the process by completely removing the unused `usedNetworks` state property from `AppStateController`. [![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** ### **Before** ### **After** ## **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. --- app/scripts/constants/sentry-state.ts | 1 - .../controllers/app-state-controller.test.ts | 11 ---- .../controllers/app-state-controller.ts | 20 ------ app/scripts/metamask-controller.js | 2 - app/scripts/migrations/134.test.ts | 62 +++++++++++++++++++ app/scripts/migrations/134.ts | 41 ++++++++++++ app/scripts/migrations/index.js | 1 + test/data/mock-send-state.json | 5 -- test/e2e/default-fixture.js | 6 -- test/e2e/fixture-builder.js | 6 -- ...rs-after-init-opt-in-background-state.json | 1 - .../errors-after-init-opt-in-ui-state.json | 1 - ...s-before-init-opt-in-background-state.json | 6 -- .../errors-before-init-opt-in-ui-state.json | 6 -- .../data/integration-init-state.json | 9 --- .../data/onboarding-completion-route.json | 1 - .../ui/new-network-info/new-network-info.js | 2 - ui/pages/routes/routes.component.test.js | 8 --- ui/pages/routes/routes.container.js | 2 - ui/selectors/selectors.js | 7 --- ui/store/actions.ts | 4 -- 21 files changed, 104 insertions(+), 98 deletions(-) create mode 100644 app/scripts/migrations/134.test.ts create mode 100644 app/scripts/migrations/134.ts diff --git a/app/scripts/constants/sentry-state.ts b/app/scripts/constants/sentry-state.ts index f18fb96d85fd..9823b2ada540 100644 --- a/app/scripts/constants/sentry-state.ts +++ b/app/scripts/constants/sentry-state.ts @@ -90,7 +90,6 @@ export const SENTRY_BACKGROUND_STATE = { termsOfUseLastAgreed: true, timeoutMinutes: true, trezorModel: true, - usedNetworks: true, }, MultichainBalancesController: { balances: false, diff --git a/app/scripts/controllers/app-state-controller.test.ts b/app/scripts/controllers/app-state-controller.test.ts index 4bc1cb63e390..f830e0ee1cdf 100644 --- a/app/scripts/controllers/app-state-controller.test.ts +++ b/app/scripts/controllers/app-state-controller.test.ts @@ -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 = { diff --git a/app/scripts/controllers/app-state-controller.ts b/app/scripts/controllers/app-state-controller.ts index 605f307ec0e4..c506dc329e94 100644 --- a/app/scripts/controllers/app-state-controller.ts +++ b/app/scripts/controllers/app-state-controller.ts @@ -62,7 +62,6 @@ export type AppStateControllerState = { hadAdvancedGasFeesSetPriorToMigration92_3: boolean; qrHardware: Json; nftsDropdownState: Json; - usedNetworks: Record; surveyLinkLastClickedOrClosed: number | null; signatureSecurityAlertResponses: Record; // States used for displaying the changed network toast @@ -138,7 +137,6 @@ type AppStateControllerInitState = Partial< AppStateControllerState, | 'qrHardware' | 'nftsDropdownState' - | 'usedNetworks' | 'surveyLinkLastClickedOrClosed' | 'signatureSecurityAlertResponses' | 'switchedNetworkDetails' @@ -184,11 +182,6 @@ const getDefaultAppStateControllerState = ( ...initState, qrHardware: {}, nftsDropdownState: {}, - usedNetworks: { - '0x1': true, - '0x5': true, - '0x539': true, - }, surveyLinkLastClickedOrClosed: null, signatureSecurityAlertResponses: {}, switchedNetworkDetails: null, @@ -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 diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index a1cbabb9bcbe..1eaf354b4caf 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -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: diff --git a/app/scripts/migrations/134.test.ts b/app/scripts/migrations/134.test.ts new file mode 100644 index 000000000000..9b3d31db017f --- /dev/null +++ b/app/scripts/migrations/134.test.ts @@ -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; + }; + } = { + 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); + }); +}); diff --git a/app/scripts/migrations/134.ts b/app/scripts/migrations/134.ts new file mode 100644 index 000000000000..e11b2abd9625 --- /dev/null +++ b/app/scripts/migrations/134.ts @@ -0,0 +1,41 @@ +import { hasProperty, isObject } from '@metamask/utils'; +import { cloneDeep } from 'lodash'; + +type VersionedData = { + meta: { version: number }; + data: Record; +}; + +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 { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + transformState(versionedData.data); + return versionedData; +} + +function transformState( + state: Record, +): Record { + if ( + hasProperty(state, 'AppStateController') && + isObject(state.AppStateController) && + hasProperty(state.AppStateController, 'usedNetworks') + ) { + console.log('Removing usedNetworks from AppStateController'); + delete state.AppStateController.usedNetworks; + } + return state; +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index e95a9bf7a9da..887732ad9dbc 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -154,6 +154,7 @@ const migrations = [ require('./131'), require('./132'), require('./133'), + require('./134'), ]; export default migrations; diff --git a/test/data/mock-send-state.json b/test/data/mock-send-state.json index 73468aca6171..f9c602808719 100644 --- a/test/data/mock-send-state.json +++ b/test/data/mock-send-state.json @@ -140,11 +140,6 @@ "isAccountMenuOpen": false, "isUnlocked": true, "completedOnboarding": true, - "usedNetworks": { - "0x1": true, - "0x5": true, - "0x539": true - }, "showTestnetMessageInDropdown": true, "alertEnabledness": { "unconnectedAccount": true diff --git a/test/e2e/default-fixture.js b/test/e2e/default-fixture.js index c2fba9d63424..a6845e40ec4c 100644 --- a/test/e2e/default-fixture.js +++ b/test/e2e/default-fixture.js @@ -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: { diff --git a/test/e2e/fixture-builder.js b/test/e2e/fixture-builder.js index 844c4766db3e..94e515d46ff6 100644 --- a/test/e2e/fixture-builder.js +++ b/test/e2e/fixture-builder.js @@ -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({ diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json index 1df430082f3e..8351032761a7 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json @@ -45,7 +45,6 @@ "nftsDropdownState": {}, "termsOfUseLastAgreed": "number", "qrHardware": {}, - "usedNetworks": { "0x1": true, "0x5": true, "0x539": true }, "snapsInstallPrivacyWarningShown": true, "surveyLinkLastClickedOrClosed": "object", "signatureSecurityAlertResponses": "object", diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json index 9988234cac2e..5d79ca5964b8 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json @@ -95,7 +95,6 @@ "nftsDropdownState": {}, "termsOfUseLastAgreed": "number", "qrHardware": {}, - "usedNetworks": { "0x1": true, "0x5": true, "0x539": true }, "snapsInstallPrivacyWarningShown": true, "surveyLinkLastClickedOrClosed": "object", "signatureSecurityAlertResponses": "object", diff --git a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json index 07b292d33b3b..1a1009c02c23 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-background-state.json @@ -42,12 +42,6 @@ "trezorModel": null, "newPrivacyPolicyToastClickedOrClosed": "boolean", "newPrivacyPolicyToastShownDate": "number", - "usedNetworks": { - "0x1": true, - "0xe708": true, - "0x5": true, - "0x539": true - }, "snapsInstallPrivacyWarningShown": true }, "CurrencyController": { diff --git a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json index f997b89bcd28..e7fe6fd2a641 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json @@ -42,12 +42,6 @@ "trezorModel": null, "newPrivacyPolicyToastClickedOrClosed": "boolean", "newPrivacyPolicyToastShownDate": "number", - "usedNetworks": { - "0x1": true, - "0xe708": true, - "0x5": true, - "0x539": true - }, "snapsInstallPrivacyWarningShown": true }, "BridgeController": { diff --git a/test/integration/data/integration-init-state.json b/test/integration/data/integration-init-state.json index 0c3853035080..ac58306adca8 100644 --- a/test/integration/data/integration-init-state.json +++ b/test/integration/data/integration-init-state.json @@ -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": [], diff --git a/test/integration/data/onboarding-completion-route.json b/test/integration/data/onboarding-completion-route.json index b2c19536a138..33d0680ab8a0 100644 --- a/test/integration/data/onboarding-completion-route.json +++ b/test/integration/data/onboarding-completion-route.json @@ -463,7 +463,6 @@ "useSafeChainsListValidation": true, "useTokenDetection": true, "useTransactionSimulations": true, - "usedNetworks": { "0x1": true, "0x5": true, "0x539": true }, "userOperations": {}, "versionFileETag": "", "versionInfo": [], diff --git a/ui/components/ui/new-network-info/new-network-info.js b/ui/components/ui/new-network-info/new-network-info.js index 15a11918afc1..9d4df0ec63ec 100644 --- a/ui/components/ui/new-network-info/new-network-info.js +++ b/ui/components/ui/new-network-info/new-network-info.js @@ -24,7 +24,6 @@ import { getParticipateInMetaMetrics, getDataCollectionForMarketing, } from '../../../selectors'; -import { setFirstTimeUsedNetwork } from '../../../store/actions'; import { PickerNetwork, Text, @@ -56,7 +55,6 @@ export default function NewNetworkInfo() { const onCloseClick = () => { setShowPopup(false); - setFirstTimeUsedNetwork(providerConfig.chainId); }; const checkTokenDetection = useCallback(async () => { diff --git a/ui/pages/routes/routes.component.test.js b/ui/pages/routes/routes.component.test.js index 14490ad6e146..e661099e1aa2 100644 --- a/ui/pages/routes/routes.component.test.js +++ b/ui/pages/routes/routes.component.test.js @@ -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, @@ -312,7 +306,6 @@ describe('toast display', () => { announcements: {}, approvalFlows: [], completedOnboarding: true, - usedNetworks: [], pendingApprovals: {}, pendingApprovalCount: 0, preferences: { @@ -341,7 +334,6 @@ describe('toast display', () => { announcements: {}, approvalFlows: [], completedOnboarding: true, - usedNetworks: [], pendingApprovals: {}, pendingApprovalCount: 0, swapsState: { swapsFeatureIsLive: true }, diff --git a/ui/pages/routes/routes.container.js b/ui/pages/routes/routes.container.js index 28f4291ee37c..0342fc2e15d1 100644 --- a/ui/pages/routes/routes.container.js +++ b/ui/pages/routes/routes.container.js @@ -8,7 +8,6 @@ import { } from '../../../shared/modules/selectors/networks'; import { getAllAccountsOnNetworkAreEmpty, - getIsNetworkUsed, getNetworkIdentifier, getPreferences, getTheme, @@ -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), diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 2baaa1a0c931..deb18c1c5309 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -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( diff --git a/ui/store/actions.ts b/ui/store/actions.ts index 9b402f476a7e..01b8b9e458d9 100644 --- a/ui/store/actions.ts +++ b/ui/store/actions.ts @@ -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]);