From a313e93842aa6c041099b4d0a8a800cf6294e31c Mon Sep 17 00:00:00 2001 From: David Murdoch <187813+davidmurdoch@users.noreply.github.com> Date: Tue, 18 Nov 2025 15:47:41 +0000 Subject: [PATCH] release(runway): cherry-pick fix: cp-13.10.0 automatically restart the extension after MM is updated in order to work around the chromium bug https://issues.chromium.org/issues/40805401 (#37552) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Automatically restart the extension after MM is updated in order to work around this chromium bug: https://issues.chromium.org/issues/40805401 It works by listening for the `onInstalled` event, and if it the `event.reason` is `"update"` and the `event.previousVersion` is NOT the current version, and the `event.previousVersion` has _not_ already been recorded in the `AppStateController`, we will reload the extension. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/37552?quickstart=1) ## **Changelog** CHANGELOG entry: Automatically restart the extension after MM is updated in order to work around this chromium bug: https://issues.chromium.org/issues/40805401 ## **Related issues** Fixes: #37503 (by hopefully making it unnecessary!) ## **Manual testing steps** ### Scenarios: #### Doesn't fire on initial installation 1. Install a version of the extension that does NOT have lavamoat (we need to use `globalThis.chrome`) and onboard 2. open the service worker/background console 3. logs starting with "[onUpdate]:" should not appear #### Doesn't reload before onboarding 1. close your browser completely (any and all Devtools panels too!) 2. re-open your browser 3. open the service_worker/background console 4. simulate an update event by running this code in the console: `chrome.runtime.onInstalled.dispatch({reason:"update", "previousVersion": "13.10.9"});` 5. should should see: `[onUpdate]: Should reload: undefined` (`undefined` indicates we didn't fetch our remote feature flags) in the logs #### Doesn't fetch feature flags until after onboarding (`Basic functionality` is left enabled during onboarding) 1. Monitor the Network tab for `https://client-config.api.cx.metamask.io/v1/flags` 2. Onboard 3. you shouldn't see https://client-config.api.cx.metamask.io/v1/flags until AFTER onboarding completes. _note: you can ensure we _don't_ fetch feature flags after onboarding by toggling `Basic functionality` to off as well._ #### Reloads after onboarding (`Basic functionality` is on) 1. close your browser completely (any and all Devtools panels too!) 2. re-open your browser 3. open the service_worker/background console 4. simulate an update event by running this code in the console: `chrome.runtime.onInstalled.dispatch({reason:"update", "previousVersion": "13.10.10"});` 5. it will reload only _once_. #### RPC Fallback still works 1. download a build from this PR (the build needs Quicknode ENV vars, which you probably don't have) 2. install and onboard 3. prevent connections to `mainnet.infura.io` however you'd like (probably via `/etc/hosts`) 4. Check the `service_worker` console and ensure quicknode URLs show up when you open the UI. ## **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/main/.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/main/.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. --- > [!NOTE] > Adds a safe-reload flow on extension update (Chromium, gated by a remote flag), defers/upgrades remote feature flag handling, and wires flags into NetworkController to enable/disable RPC failover; updates messengers, tests, and fixtures. > > - **Background**: > - Add `onUpdate` handler to record update metadata and trigger safe reload on Chromium when `extensionPlatformAutoReloadAfterUpdate` flag is true. > - Handle `runtime.onInstalled` update path asynchronously and invoke `onUpdate(controller, platform, previousVersion, requestSafeReload)`. > - Lazily fetch remote feature flags on UI open via `updateRemoteFeatureFlags` util. > - Simplify `setupController` signature (stop passing `requestSafeReload`). > - **Remote Feature Flags**: > - Initialize `RemoteFeatureFlagController` with persisted state; gate enablement by onboarding completion and `useExternalServices`. > - Subscribe to `PreferencesController` and `OnboardingController` state changes to enable/disable and refresh flags. > - Extend messengers to allow `RemoteFeatureFlagController:getState` and related `stateChange` events. > - **Network**: > - Use remote flag `walletFrameworkRpcFailoverEnabled` to set `NetworkController` `isRpcFailoverEnabled` at init and toggle on flag changes. > - **Tests/Fixtures**: > - Update unit tests to cover new messenger actions and RPC failover gating. > - Add `FixtureBuilder.withRemoteFeatureFlags` and set flags/manifest in e2e tests. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 802612e512cbc6a8b73b7263a22cd5643feda065. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --------- Co-authored-by: Ganesh Suresh Patra Co-authored-by: Mark Stacey Co-authored-by: Mark Stacey --- app/scripts/background.js | 72 ++++----------- .../network-controller-messenger.ts | 9 +- ...emote-feature-flag-controller-messenger.ts | 17 +++- .../network-controller-init.test.ts | 34 ++++++-- .../network-controller-init.ts | 22 ++++- ...emote-feature-flag-controller-init.test.ts | 11 ++- .../remote-feature-flag-controller-init.ts | 73 +++++++++++++--- .../lib/update-remote-feature-flags.ts | 24 +++++ app/scripts/metamask-controller.js | 4 +- app/scripts/on-update.ts | 87 +++++++++++++++++++ test/e2e/fixture-builder.js | 8 ++ .../identity/account-syncing/balances.spec.ts | 10 ++- .../tests/privacy/basic-functionality.spec.ts | 7 ++ 13 files changed, 294 insertions(+), 84 deletions(-) create mode 100644 app/scripts/lib/update-remote-feature-flags.ts create mode 100644 app/scripts/on-update.ts diff --git a/app/scripts/background.js b/app/scripts/background.js index ff535c8c1e17..0b7f2f438153 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -63,6 +63,7 @@ import ExtensionStore from './lib/stores/extension-store'; import ReadOnlyNetworkStore from './lib/stores/read-only-network-store'; import migrations from './migrations'; import Migrator from './lib/migrator'; +import { updateRemoteFeatureFlags } from './lib/update-remote-feature-flags'; import ExtensionPlatform from './platforms/extension'; import { SENTRY_BACKGROUND_STATE } from './constants/sentry-state'; @@ -78,6 +79,7 @@ import { getPlatform, shouldEmitDappViewedEvent } from './lib/util'; import { createOffscreen } from './offscreen'; import { setupMultiplex } from './lib/stream-utils'; import rawFirstTimeState from './first-time-state'; +import { onUpdate } from './on-update'; /* eslint-enable import/first */ @@ -118,6 +120,9 @@ const localStore = useReadOnlyNetworkStore ? new ReadOnlyNetworkStore() : new ExtensionStore(); const persistenceManager = new PersistenceManager({ localStore }); + +const { update, requestSafeReload } = getRequestSafeReload(persistenceManager); + // Setup global hook for improved Sentry state snapshots during initialization global.stateHooks.getMostRecentPersistedState = () => persistenceManager.mostRecentRetrievedState; @@ -721,9 +726,6 @@ async function initialize(backup) { const cronjobControllerStorageManager = new CronjobControllerStorageManager(); await cronjobControllerStorageManager.init(); - const { update, requestSafeReload } = - getRequestSafeReload(persistenceManager); - setupController( initState, initLangCode, @@ -732,7 +734,6 @@ async function initialize(backup) { initData.meta, offscreenPromise, preinstalledSnaps, - requestSafeReload, cronjobControllerStorageManager, ); @@ -1102,7 +1103,6 @@ function trackAppOpened(environment) { * @param {object} stateMetadata - Metadata about the initial state and migrations, including the most recent migration version * @param {Promise} offscreenPromise - A promise that resolves when the offscreen document has finished initialization. * @param {Array} preinstalledSnaps - A list of preinstalled Snaps loaded from disk during boot. - * @param {() => Promise)} requestSafeReload - A function that requests a safe reload of the extension. * @param {CronjobControllerStorageManager} cronjobControllerStorageManager - A storage manager for the CronjobController. */ export function setupController( @@ -1113,7 +1113,6 @@ export function setupController( stateMetadata, offscreenPromise, preinstalledSnaps, - requestSafeReload, cronjobControllerStorageManager, ) { // @@ -1241,7 +1240,8 @@ export function setupController( controller.setupTrustedCommunication(portStream, remotePort.sender); trackAppOpened(processName); - initializeRemoteFeatureFlags(); + // lazily update the remote feature flags every time the UI is opened. + updateRemoteFeatureFlags(controller); if (processName === ENVIRONMENT_TYPE_POPUP) { openPopupCount += 1; @@ -1480,22 +1480,6 @@ export function setupController( } } - /** - * Initializes remote feature flags by making a request to fetch them from the clientConfigApi. - * This function is called when MM is during internal process. - * If the request fails, the error will be logged but won't interrupt extension initialization. - * - * @returns {Promise} A promise that resolves when the remote feature flags have been updated. - */ - async function initializeRemoteFeatureFlags() { - try { - // initialize the request to fetch remote feature flags - await controller.remoteFeatureFlagController.updateRemoteFeatureFlags(); - } catch (error) { - log.error('Error initializing remote feature flags:', error); - } - } - function getPendingApprovalCount() { try { const pendingApprovalCount = @@ -1650,15 +1634,16 @@ const addAppInstalledEvent = () => { * * @param {[chrome.runtime.InstalledDetails]} params - Array containing a single installation details object. */ -function handleOnInstalled([details]) { +async function handleOnInstalled([details]) { if (details.reason === 'install') { onInstall(); - } else if ( - details.reason === 'update' && - details.previousVersion && - details.previousVersion !== platform.getVersion() - ) { - onUpdate(details.previousVersion); + } else if (details.reason === 'update') { + const { previousVersion } = details; + if (!previousVersion || previousVersion === platform.getVersion()) { + return; + } + await isInitialized; + onUpdate(controller, platform, previousVersion, requestSafeReload); } } @@ -1694,33 +1679,6 @@ if ( }); } -// // On first install, open a new tab with MetaMask -// async function onInstall() { -// const storeAlreadyExisted = Boolean(await localStore.get()); -// // If the store doesn't exist, then this is the first time running this script, -// // and is therefore an install -// if (process.env.IN_TEST) { -// addAppInstalledEvent(); -// } else if (!storeAlreadyExisted && !process.env.METAMASK_DEBUG) { -// addAppInstalledEvent(); -// platform.openExtensionInBrowser(); -// } -// } - -/** - * Trigger actions that should happen only upon update installation - * - * @param previousVersion - */ -async function onUpdate(previousVersion) { - await isInitialized; - log.debug('Update installation detected'); - controller.appStateController.setLastUpdatedAt(Date.now()); - if (previousVersion) { - controller.appStateController.setLastUpdatedFromVersion(previousVersion); - } -} - /** * Trigger actions that should happen only when an update is available */ diff --git a/app/scripts/controller-init/messengers/network-controller-messenger.ts b/app/scripts/controller-init/messengers/network-controller-messenger.ts index dca8d90e8921..de84ce7ac161 100644 --- a/app/scripts/controller-init/messengers/network-controller-messenger.ts +++ b/app/scripts/controller-init/messengers/network-controller-messenger.ts @@ -5,7 +5,10 @@ import { NetworkControllerRpcEndpointDegradedEvent, NetworkControllerRpcEndpointUnavailableEvent, } from '@metamask/network-controller'; -import { RemoteFeatureFlagControllerState } from '@metamask/remote-feature-flag-controller'; +import { + RemoteFeatureFlagControllerGetStateAction, + RemoteFeatureFlagControllerState, +} from '@metamask/remote-feature-flag-controller'; import { MetaMetricsControllerGetMetaMetricsIdAction, MetaMetricsControllerTrackEventAction, @@ -45,7 +48,8 @@ export function getNetworkControllerMessenger( type AllowedInitializationActions = | MetaMetricsControllerGetMetaMetricsIdAction - | MetaMetricsControllerTrackEventAction; + | MetaMetricsControllerTrackEventAction + | RemoteFeatureFlagControllerGetStateAction; type AllowedInitializationEvents = | NetworkControllerRpcEndpointUnavailableEvent @@ -86,6 +90,7 @@ export function getNetworkControllerInitMessenger( actions: [ 'MetaMetricsController:getMetaMetricsId', 'MetaMetricsController:trackEvent', + 'RemoteFeatureFlagController:getState', ], events: [ 'NetworkController:rpcEndpointUnavailable', diff --git a/app/scripts/controller-init/messengers/remote-feature-flag-controller-messenger.ts b/app/scripts/controller-init/messengers/remote-feature-flag-controller-messenger.ts index a38536546643..d1e0bd25be84 100644 --- a/app/scripts/controller-init/messengers/remote-feature-flag-controller-messenger.ts +++ b/app/scripts/controller-init/messengers/remote-feature-flag-controller-messenger.ts @@ -5,6 +5,10 @@ import { PreferencesControllerStateChangeEvent, } from '../../controllers/preferences-controller'; import { RootMessenger } from '../../lib/messenger'; +import type { + OnboardingControllerGetStateAction, + OnboardingControllerStateChangeEvent, +} from '../../controllers/onboarding'; export type RemoteFeatureFlagControllerMessenger = ReturnType< typeof getRemoteFeatureFlagControllerMessenger @@ -33,9 +37,12 @@ export function getRemoteFeatureFlagControllerMessenger( type AllowedInitializationActions = | MetaMetricsControllerGetMetaMetricsIdAction - | PreferencesControllerGetStateAction; + | PreferencesControllerGetStateAction + | OnboardingControllerGetStateAction; -type AllowedInitializationEvents = PreferencesControllerStateChangeEvent; +type AllowedInitializationEvents = + | PreferencesControllerStateChangeEvent + | OnboardingControllerStateChangeEvent; export type RemoteFeatureFlagControllerInitMessenger = ReturnType< typeof getRemoteFeatureFlagControllerInitMessenger @@ -68,8 +75,12 @@ export function getRemoteFeatureFlagControllerInitMessenger( actions: [ 'MetaMetricsController:getMetaMetricsId', 'PreferencesController:getState', + 'OnboardingController:getState', + ], + events: [ + 'PreferencesController:stateChange', + 'OnboardingController:stateChange', ], - events: ['PreferencesController:stateChange'], }); return controllerInitMessenger; } diff --git a/app/scripts/controller-init/network-controller-init.test.ts b/app/scripts/controller-init/network-controller-init.test.ts index f12ab7b6ad8a..eeb05adb678f 100644 --- a/app/scripts/controller-init/network-controller-init.test.ts +++ b/app/scripts/controller-init/network-controller-init.test.ts @@ -1,12 +1,15 @@ import { ControllerStateChangeEvent } from '@metamask/base-controller'; import { + ActionConstraint, MOCK_ANY_NAMESPACE, Messenger, MockAnyNamespace, } from '@metamask/messenger'; import { NetworkController } from '@metamask/network-controller'; -import { RemoteFeatureFlagControllerState } from '@metamask/remote-feature-flag-controller'; -import { getRootMessenger } from '../lib/messenger'; +import { + RemoteFeatureFlagControllerGetStateAction, + RemoteFeatureFlagControllerState, +} from '@metamask/remote-feature-flag-controller'; import { ControllerInitRequest } from './types'; import { buildControllerInitRequestMock } from './test/utils'; import { @@ -37,17 +40,33 @@ jest.mock('@metamask/network-controller', () => { }); function getInitRequestMock( - baseMessenger = getRootMessenger(), + messenger = new Messenger< + MockAnyNamespace, + RemoteFeatureFlagControllerGetStateAction | ActionConstraint, + ControllerStateChangeEvent< + 'RemoteFeatureFlagController', + RemoteFeatureFlagControllerState + > + >({ namespace: MOCK_ANY_NAMESPACE }), ): jest.Mocked< ControllerInitRequest< NetworkControllerMessenger, NetworkControllerInitMessenger > > { + messenger.registerActionHandler( + 'RemoteFeatureFlagController:getState', + jest.fn().mockReturnValue({ + remoteFeatureFlags: { + walletFrameworkRpcFailoverEnabled: true, + }, + }), + ); + const requestMock = { ...buildControllerInitRequestMock(), - controllerMessenger: getNetworkControllerMessenger(baseMessenger), - initMessenger: getNetworkControllerInitMessenger(baseMessenger), + controllerMessenger: getNetworkControllerMessenger(messenger), + initMessenger: getNetworkControllerInitMessenger(messenger), }; return requestMock; @@ -78,6 +97,7 @@ describe('NetworkControllerInit', () => { getBlockTrackerOptions: expect.any(Function), getRpcServiceOptions: expect.any(Function), infuraProjectId: undefined, + isRpcFailoverEnabled: true, }); }); @@ -330,7 +350,7 @@ describe('NetworkControllerInit', () => { it('enables RPC failover when the `walletFrameworkRpcFailoverEnabled` feature flag is enabled', () => { const messenger = new Messenger< MockAnyNamespace, - never, + RemoteFeatureFlagControllerGetStateAction, ControllerStateChangeEvent< 'RemoteFeatureFlagController', RemoteFeatureFlagControllerState @@ -359,7 +379,7 @@ describe('NetworkControllerInit', () => { it('disables RPC failover when the `walletFrameworkRpcFailoverEnabled` feature flag is disabled', () => { const messenger = new Messenger< MockAnyNamespace, - never, + RemoteFeatureFlagControllerGetStateAction, ControllerStateChangeEvent< 'RemoteFeatureFlagController', RemoteFeatureFlagControllerState diff --git a/app/scripts/controller-init/network-controller-init.ts b/app/scripts/controller-init/network-controller-init.ts index b043fca79148..0b43cd2fa8ee 100644 --- a/app/scripts/controller-init/network-controller-init.ts +++ b/app/scripts/controller-init/network-controller-init.ts @@ -10,6 +10,7 @@ import { ChainId, } from '@metamask/controller-utils'; import { hasProperty } from '@metamask/utils'; +import { RemoteFeatureFlagControllerState } from '@metamask/remote-feature-flag-controller'; import { SECOND } from '../../../shared/constants/time'; import { getIsQuicknodeEndpointUrl } from '../../../shared/lib/network-utils'; import { @@ -167,8 +168,24 @@ export const NetworkControllerInit: ControllerInitFunction< initMessenger, persistedState, }) => { + const remoteFeatureFlagsControllerState = initMessenger.call( + 'RemoteFeatureFlagController:getState', + ); const initialState = getInitialState(persistedState.NetworkController); + /** + * Determines if RPC failover is enabled based on RemoteFeatureFlagController + * state. + * + * @param state - RemoteFeatureFlagControllerState + * @returns true if RPC failover is enabled, false otherwise + */ + const getIsRpcFailoverEnabled = (state: RemoteFeatureFlagControllerState) => { + const walletFrameworkRpcFailoverEnabled = state.remoteFeatureFlags + .walletFrameworkRpcFailoverEnabled as boolean | undefined; + return walletFrameworkRpcFailoverEnabled ?? false; + }; + const getBlockTrackerOptions = () => { return process.env.IN_TEST ? {} @@ -233,6 +250,9 @@ export const NetworkControllerInit: ControllerInitFunction< getBlockTrackerOptions, getRpcServiceOptions, additionalDefaultNetworks: ADDITIONAL_DEFAULT_NETWORKS, + isRpcFailoverEnabled: getIsRpcFailoverEnabled( + remoteFeatureFlagsControllerState, + ), }); initMessenger.subscribe( @@ -284,7 +304,7 @@ export const NetworkControllerInit: ControllerInitFunction< controller.disableRpcFailover(); } }, - (state) => state.remoteFeatureFlags.walletFrameworkRpcFailoverEnabled, + getIsRpcFailoverEnabled, ); // Delay lookupNetwork until after onboarding to prevent network requests before the user can diff --git a/app/scripts/controller-init/remote-feature-flag-controller-init.test.ts b/app/scripts/controller-init/remote-feature-flag-controller-init.test.ts index 014b04d32b51..7958845321aa 100644 --- a/app/scripts/controller-init/remote-feature-flag-controller-init.test.ts +++ b/app/scripts/controller-init/remote-feature-flag-controller-init.test.ts @@ -10,6 +10,7 @@ import { } from '@metamask/messenger'; import { ENVIRONMENT } from '../../../development/build/constants'; import { PreferencesControllerGetStateAction } from '../controllers/preferences-controller'; +import { OnboardingControllerGetStateAction } from '../controllers/onboarding'; import { getConfigForRemoteFeatureFlagRequest, RemoteFeatureFlagControllerInit, @@ -33,7 +34,9 @@ function getInitRequestMock(): jest.Mocked< > { const baseMessenger = new Messenger< MockAnyNamespace, - PreferencesControllerGetStateAction | ActionConstraint, + | PreferencesControllerGetStateAction + | OnboardingControllerGetStateAction + | ActionConstraint, never >({ namespace: MOCK_ANY_NAMESPACE, @@ -45,6 +48,12 @@ function getInitRequestMock(): jest.Mocked< useExternalServices: true, }), ); + baseMessenger.registerActionHandler( + 'OnboardingController:getState', + jest.fn().mockReturnValue({ + completedOnboarding: true, + }), + ); const requestMock = { ...buildControllerInitRequestMock(), diff --git a/app/scripts/controller-init/remote-feature-flag-controller-init.ts b/app/scripts/controller-init/remote-feature-flag-controller-init.ts index 58dfaaee2bdb..6c590d6cdc76 100644 --- a/app/scripts/controller-init/remote-feature-flag-controller-init.ts +++ b/app/scripts/controller-init/remote-feature-flag-controller-init.ts @@ -58,20 +58,35 @@ export function getConfigForRemoteFeatureFlagRequest() { * @param request - The request object. * @param request.controllerMessenger - The messenger to use for the controller. * @param request.initMessenger - The messenger to use for initialization. + * @param request.persistedState - The persisted state of the extension. * @returns The initialized controller. */ export const RemoteFeatureFlagControllerInit: ControllerInitFunction< RemoteFeatureFlagController, RemoteFeatureFlagControllerMessenger, RemoteFeatureFlagControllerInitMessenger -> = ({ controllerMessenger, initMessenger }) => { +> = ({ controllerMessenger, initMessenger, persistedState }) => { + const onboardingState = initMessenger.call('OnboardingController:getState'); const preferencesState = initMessenger.call('PreferencesController:getState'); const { distribution, environment } = getConfigForRemoteFeatureFlagRequest(); + let canUseExternalServices = preferencesState.useExternalServices === true; + let hasCompletedOnboarding = onboardingState.completedOnboarding === true; + + /** + * Uses state from multiple controllers to determine if the remote feature flag + * controller should be disabled or not. + * + * @returns `true` if it should be disabled, `false` otherwise. + */ + const getIsDisabled = () => + !hasCompletedOnboarding || !canUseExternalServices; + const controller = new RemoteFeatureFlagController({ + state: persistedState.RemoteFeatureFlagController, messenger: controllerMessenger, fetchInterval: 15 * 60 * 1000, // 15 minutes in milliseconds - disabled: !preferencesState.useExternalServices, + disabled: getIsDisabled(), getMetaMetricsId: () => initMessenger.call('MetaMetricsController:getMetaMetricsId'), clientConfigApiService: new ClientConfigApiService({ @@ -84,24 +99,62 @@ export const RemoteFeatureFlagControllerInit: ControllerInitFunction< }), }); + /** + * Enables or disables the controller based on the current state of other + * controllers. + */ + function toggle() { + const shouldBeDisabled = getIsDisabled(); + if (shouldBeDisabled) { + controller.disable(); + } else { + controller.enable(); + controller.updateRemoteFeatureFlags().catch((error) => { + console.error('Failed to update remote feature flags:', error); + }); + } + } + + /** + * Subscribe to relevant state changes in the Onboarding Controller + * to collect information that helps determine if we can fetch remote + * feature flags. + */ initMessenger.subscribe( 'PreferencesController:stateChange', previousValueComparator((prevState, currState) => { const { useExternalServices: prevUseExternalServices } = prevState; const { useExternalServices: currUseExternalServices } = currState; - if (currUseExternalServices && !prevUseExternalServices) { - controller.enable(); - controller.updateRemoteFeatureFlags().catch((error) => { - console.error('Failed to update remote feature flags:', error); - }); - } else if (!currUseExternalServices && prevUseExternalServices) { - controller.disable(); + const hasChanged = currUseExternalServices !== prevUseExternalServices; + if (hasChanged) { + canUseExternalServices = currUseExternalServices === true; + toggle(); } - return true; }, preferencesState), ); + /** + * Subscribe to relevant state changes in the Onboarding Controller + * to collect information that helps determine if we can fetch remote + * feature flags. + */ + initMessenger.subscribe( + 'OnboardingController:stateChange', + previousValueComparator((prevState, currState) => { + const { completedOnboarding: prevCompletedOnboarding } = prevState; + const { completedOnboarding: currCompletedOnboarding } = currState; + // yes, it is possible for completedOnboarding to change back to `false` + // after it has been `true` + const hasChanged = currCompletedOnboarding !== prevCompletedOnboarding; + if (hasChanged) { + hasCompletedOnboarding = currCompletedOnboarding === true; + toggle(); + } + return true; + }, onboardingState), + ); + return { controller, }; diff --git a/app/scripts/lib/update-remote-feature-flags.ts b/app/scripts/lib/update-remote-feature-flags.ts new file mode 100644 index 000000000000..8381a3e6b543 --- /dev/null +++ b/app/scripts/lib/update-remote-feature-flags.ts @@ -0,0 +1,24 @@ +import log from 'loglevel'; +import { RemoteFeatureFlagController } from '@metamask/remote-feature-flag-controller'; +import MetamaskController from '../metamask-controller'; + +/** + * Updates remote feature flags by making a request to fetch them from the clientConfigApi. + * This function is called when MM is initially loaded, as well as when our UI is opened. + * If the request fails, the error will be logged but won't interrupt extension initialization. + * + * @param metamaskController - The MetaMask controller instance. + * @returns A promise that resolves when the remote feature flags have been updated. + */ +export async function updateRemoteFeatureFlags( + metamaskController: MetamaskController, +): Promise { + try { + const remoteController: RemoteFeatureFlagController = + metamaskController.remoteFeatureFlagController; + // initialize the request to fetch remote feature flags + await remoteController.updateRemoteFeatureFlags(); + } catch (error) { + log.error('Error initializing remote feature flags:', error); + } +} diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 9c7b8b6d55ea..0bf9636830cc 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -584,10 +584,11 @@ export default class MetamaskController extends EventEmitter { PermissionLogController: PermissionLogControllerInit, SubjectMetadataController: SubjectMetadataControllerInit, AppStateController: AppStateControllerInit, + OnboardingController: OnboardingControllerInit, + RemoteFeatureFlagController: RemoteFeatureFlagControllerInit, NetworkController: NetworkControllerInit, MetaMetricsController: MetaMetricsControllerInit, MetaMetricsDataDeletionController: MetaMetricsDataDeletionControllerInit, - RemoteFeatureFlagController: RemoteFeatureFlagControllerInit, GasFeeController: GasFeeControllerInit, UserOperationController: UserOperationControllerInit, ExecutionService: ExecutionServiceInit, @@ -604,7 +605,6 @@ export default class MetamaskController extends EventEmitter { AccountActivityService: AccountActivityServiceInit, PPOMController: PPOMControllerInit, PhishingController: PhishingControllerInit, - OnboardingController: OnboardingControllerInit, AccountTrackerController: AccountTrackerControllerInit, TransactionController: TransactionControllerInit, SmartTransactionsController: SmartTransactionsControllerInit, diff --git a/app/scripts/on-update.ts b/app/scripts/on-update.ts new file mode 100644 index 000000000000..2bb4ee4f510b --- /dev/null +++ b/app/scripts/on-update.ts @@ -0,0 +1,87 @@ +import log from 'loglevel'; +import { RemoteFeatureFlagController } from '@metamask/remote-feature-flag-controller'; +import { PLATFORM_FIREFOX } from '../../shared/constants/app'; +import { getPlatform } from './lib/util'; +import type MetaMaskController from './metamask-controller'; +import type ExtensionPlatform from './platforms/extension'; +import { AppStateController } from './controllers/app-state-controller'; +/** + * Trigger actions that should happen only upon update installation. Calling + * this might result in the extension restarting on Chromium-based browsers. + * + * @param controller - The MetaMask controller instance. + * @param controller.store - The MetaMask store. + * @param controller.remoteFeatureFlagController - The remote feature flag + * controller. + * @param controller.appStateController - The app state controller. + * @param platform - The ExtensionPlatform API. + * @param previousVersion - The previous version string. + * @param requestSafeReload - A function to request a safe reload of the + * extension background process. + */ +export function onUpdate( + // we use a custom type here because the `MetaMaskController` type doesn't + // include the actual controllers as properties. + controller: { + store: MetaMaskController['store']; + remoteFeatureFlagController: RemoteFeatureFlagController; + appStateController: AppStateController; + }, + platform: ExtensionPlatform, + previousVersion: string, + requestSafeReload: () => void, +): void { + const { appStateController } = controller; + const { lastUpdatedFromVersion } = appStateController.state; + const isFirefox = getPlatform() === PLATFORM_FIREFOX; + + log.debug('[onUpdate]: Update installation detected'); + log.info(`[onUpdate]: Updated from version ${previousVersion}`); + log.info( + `[onUpdate]: Recorded last updated from version: ${lastUpdatedFromVersion}`, + ); + log.info(`[onUpdate]: isFirefox: ${isFirefox}`); + log.info(`[onUpdate]: Current version: ${platform.getVersion()}`); + + // Browser might trigger an update event even when the version hasn't changed, + // like when reloading the extension manually. + if (previousVersion === lastUpdatedFromVersion) { + return; + } + + const lastUpdatedAt = Date.now(); + + appStateController.setLastUpdatedAt(lastUpdatedAt); + appStateController.setLastUpdatedFromVersion(previousVersion); + + if (!isFirefox) { + // Work around Chromium bug https://issues.chromium.org/issues/40805401 + // by doing a safe reload after an update. We'll be able to gate this + // behind a Chromium version check once we know the chromium version # + // that fixes this bug, ETA: December 2025 (likely in `143.0.7465.0`). + // Once we no longer support the affected Chromium versions, we should + // remove this workaround. + // We only want to do the safe reload when the version actually changed, + // just as a safe guard, as Chrome fires this event each time we call + // `runtime.reload` -- as we really don't want to send Chrome into a restart + // loop! This is overkill, as `onUpdate` is already only called when + // `previousVersion !== platform.getVersion()`, but better safe than better + // safe than better safe than... rebooting forever. :-) + const { remoteFeatureFlagController } = controller; + const shouldReload = remoteFeatureFlagController.state.remoteFeatureFlags + .extensionPlatformAutoReloadAfterUpdate as boolean | undefined; + log.info(`[onUpdate]: Should reload: ${shouldReload}`); + if (shouldReload === true) { + log.info( + `[onUpdate]: Requesting "safe reload" after update to ${platform.getVersion()}`, + ); + // use `setImmediate` to be absolutely sure the reload happens after + // other "update" events triggered by the `setLastUpdatedFromVersion` + // and `setLastUpdatedAt` calls above have been processed by storage. + // I think there _is_ still a risk of a race condition here, mostly + // due to the complexity of state storage's locks, debounce, and async + // nature. + setImmediate(requestSafeReload); + } + } +} diff --git a/test/e2e/fixture-builder.js b/test/e2e/fixture-builder.js index a66abca0909f..02d4cca39ba8 100644 --- a/test/e2e/fixture-builder.js +++ b/test/e2e/fixture-builder.js @@ -1916,6 +1916,14 @@ class FixtureBuilder { return this; } + withRemoteFeatureFlags(remoteFeatureFlags = {}) { + merge( + this.fixture.data.RemoteFeatureFlagController.remoteFeatureFlags, + remoteFeatureFlags, + ); + return this; + } + build() { if (!this.fixture.meta) { this.fixture.meta = { diff --git a/test/e2e/tests/identity/account-syncing/balances.spec.ts b/test/e2e/tests/identity/account-syncing/balances.spec.ts index f973637634ae..a9bd74926284 100644 --- a/test/e2e/tests/identity/account-syncing/balances.spec.ts +++ b/test/e2e/tests/identity/account-syncing/balances.spec.ts @@ -97,7 +97,15 @@ describe('Account syncing - Accounts with Balances', function () { await withFixtures( { - fixtures: new FixtureBuilder({ onboarding: true }).build(), + fixtures: new FixtureBuilder({ onboarding: true }) + .withRemoteFeatureFlags({ + enableMultichainAccountsState2: { + enabled: true, + featureVersion: '2', + minimumVersion: '13.5.0', + }, + }) + .build(), title: this.test?.fullTitle(), testSpecificMock: phase2MockSetup, }, diff --git a/test/e2e/tests/privacy/basic-functionality.spec.ts b/test/e2e/tests/privacy/basic-functionality.spec.ts index 48add00b1543..717f582c193a 100644 --- a/test/e2e/tests/privacy/basic-functionality.spec.ts +++ b/test/e2e/tests/privacy/basic-functionality.spec.ts @@ -106,6 +106,13 @@ describe('MetaMask onboarding', function () { { fixtures: new FixtureBuilder({ onboarding: true }).build(), title: this.test?.fullTitle(), + manifestFlags: { + remoteFeatureFlags: { + sendRedesign: { + enabled: false, + }, + }, + }, testSpecificMock: (server: Mockttp) => mockApis( server,