From f83a819400c91a67277aaee24328126e2b742aec Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 26 Nov 2024 19:40:29 -0330 Subject: [PATCH] chore: Update `@metamask/gas-fee-controller` and peer deps (#28745) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update the `@metamask/gas-fee-controller` package to v21 to satisfy the peer dependency on `@metamask/network-controller@^21.0.0`, and update the `@metamask/user-operation-controller` to satisfy its peer dependency upon `@metamask/gas-fee-controller`. Note that an older version of `@metamask/gas-fee-controller` (v18) remains in the dependency tree, but only because it's imported by `@metamask/smart-transaction-controller` for type reasons. It has no runtime impact on the application, so the associated peer dependency warnings from this older release can be ignored. This will be eliminated soon, in an upcoming PR. The updated `@metamask/user-operation-controller` still does not have its peer dependencies satisfied, but the problems are pre-existing. The `@metamask/keyring-controller` and `@metamask/transaction-controller` packages are head of where this package expects them to be. This is not made worse by this PR though, and will be addressed in a future PR. Changelogs: - `@metamask/gas-fee-controller@21`: https://github.com/MetaMask/core/blob/main/packages/gas-fee-controller/CHANGELOG.md#2100 - One breaking change with an impact: >The inherited AbstractPollingController method startPollingByNetworkClientId has been renamed to startPolling (https://github.com/MetaMask/core/pull/4752) - `@metamask/user-operation-controller@16`: https://github.com/MetaMask/core/blob/main/packages/user-operation-controller/CHANGELOG.md#1600 - No breaking changes with an impact. It appeared at first that the `startPollingByNetworkClientId` was breaking, but the user operation controller had an override for that method so it was preserved ([see here](https://github.com/MetaMask/core/pull/4752/files#diff-335af05ece636eb593b348e369dff139dfbfea49ad4e9ba3bb47b4909aab9aefR304)) [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28745?quickstart=1) Related to https://github.com/MetaMask/MetaMask-planning/issues/3568 N/A N/A - [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. - [ ] 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. --------- Co-authored-by: MetaMask Bot --- app/scripts/background.js | 5 +- app/scripts/constants/sentry-state.ts | 1 + app/scripts/metamask-controller.js | 64 +++++++++++ app/scripts/metamask-controller.test.js | 104 ++++++++++++++++++ lavamoat/browserify/beta/policy.json | 10 ++ lavamoat/browserify/flask/policy.json | 10 ++ lavamoat/browserify/main/policy.json | 10 ++ lavamoat/browserify/mmi/policy.json | 10 ++ lavamoat/build-system/policy.json | 10 +- package.json | 1 + privacy-snapshot.json | 3 +- test/e2e/fixture-builder.js | 6 + test/e2e/mock-e2e.js | 16 ++- .../tests/remote-feature-flag/mock-data.ts | 3 + .../remote-feature-flag.spec.ts | 42 +++++++ ui/selectors/selectors.js | 4 + yarn.lock | 12 ++ 17 files changed, 307 insertions(+), 4 deletions(-) create mode 100644 test/e2e/tests/remote-feature-flag/mock-data.ts create mode 100644 test/e2e/tests/remote-feature-flag/remote-feature-flag.spec.ts diff --git a/app/scripts/background.js b/app/scripts/background.js index 2b2f5c4693df..eb9019a9573c 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -772,7 +772,6 @@ export function setupController( // // MetaMask Controller // - controller = new MetamaskController({ infuraProjectId: process.env.INFURA_PROJECT_ID, // User confirmation callbacks: @@ -885,12 +884,16 @@ export function setupController( senderUrl?.origin === `chrome-extension://${browser.runtime.id}`; } + controller.remoteFeatureFlagController.getRemoteFeatureFlags(); + if (isMetaMaskInternalProcess) { const portStream = overrides?.getPortStream?.(remotePort) || new PortStream(remotePort); // communication with popup controller.isClientOpen = true; controller.setupTrustedCommunication(portStream, remotePort.sender); + // initialize the request to fetch remote feature flags + controller.remoteFeatureFlagController.getRemoteFeatureFlags(); if (processName === ENVIRONMENT_TYPE_POPUP) { openPopupCount += 1; diff --git a/app/scripts/constants/sentry-state.ts b/app/scripts/constants/sentry-state.ts index f18fb96d85fd..2dd7ef5db159 100644 --- a/app/scripts/constants/sentry-state.ts +++ b/app/scripts/constants/sentry-state.ts @@ -271,6 +271,7 @@ export const SENTRY_BACKGROUND_STATE = { useTransactionSimulations: true, enableMV3TimestampSave: true, }, + FeatureFlagController: {}, NotificationServicesPushController: { fcmToken: false, }, diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index d60d937e1c3c..7bc5d646baab 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -104,6 +104,10 @@ import { } from '@metamask/controller-utils'; import { AccountsController } from '@metamask/accounts-controller'; +import { + RemoteFeatureFlagController, + ClientConfigApiService, +} from '@metamask/remote-feature-flag-controller'; ///: BEGIN:ONLY_INCLUDE_IF(build-mmi) import { @@ -244,6 +248,7 @@ import { endTrace, trace } from '../../shared/lib/trace'; // eslint-disable-next-line import/no-restricted-paths import { isSnapId } from '../../ui/helpers/utils/snaps'; import { BridgeStatusAction } from '../../shared/types/bridge-status'; +import { ENVIRONMENT } from '../../development/build/constants'; import fetchWithCache from '../../shared/lib/fetch-with-cache'; import { BalancesController as MultichainBalancesController } from './lib/accounts/BalancesController'; import { @@ -396,6 +401,17 @@ const PHISHING_SAFELIST = 'metamask-phishing-safelist'; // OneKey devices can connect to Metamask using Trezor USB transport. They use a specific device minor version (99) to differentiate between genuine Trezor and OneKey devices. export const ONE_KEY_VIA_TREZOR_MINOR_VERSION = 99; +const environmentMappingForRemoteFeatureFlag = { + [ENVIRONMENT.DEVELOPMENT]: 'dev', + [ENVIRONMENT.RELEASE_CANDIDATE]: 'rc', + [ENVIRONMENT.PRODUCTION]: 'prod', +}; + +const buildTypeMappingForRemoteFeatureFlag = { + flask: 'flask', + main: 'main', +}; + export default class MetamaskController extends EventEmitter { /** * @param {object} opts @@ -2352,6 +2368,41 @@ export default class MetamaskController extends EventEmitter { clearPendingConfirmations.bind(this), ); + // RemoteFeatureFlagController has subscription for preferences changes + this.controllerMessenger.subscribe( + 'PreferencesController:stateChange', + previousValueComparator((prevState, currState) => { + const { useExternalServices: prevUseExternalServices } = prevState; + const { useExternalServices: currUseExternalServices } = currState; + + if (currUseExternalServices && !prevUseExternalServices) { + this.remoteFeatureFlagController.enable(); + this.getRemoteFeatureFlags(); + } else if (!currUseExternalServices && prevUseExternalServices) { + this.remoteFeatureFlagController.disable(); + } + }, this.preferencesController.state), + ); + + // Initialize RemoteFeatureFlagController + this.remoteFeatureFlagController = new RemoteFeatureFlagController({ + messenger: this.controllerMessenger.getRestricted({ + name: 'RemoteFeatureFlagController', + allowedActions: ['PreferencesController:getState'], + allowedEvents: ['PreferencesController:stateChange'], + }), + disabled: !this.preferencesController.state.useExternalServices, + clientConfigApiService: new ClientConfigApiService({ + fetch: globalThis.fetch.bind(globalThis), + config: { + client: 'extension', + distribution: + this._getConfigForRemoteFeatureFlagRequest().distribution, + environment: this._getConfigForRemoteFeatureFlagRequest().environment, + }, + }), + }); + this.metamaskMiddleware = createMetamaskMiddleware({ static: { eth_syncing: false, @@ -2518,6 +2569,7 @@ export default class MetamaskController extends EventEmitter { NotificationServicesController: this.notificationServicesController, NotificationServicesPushController: this.notificationServicesPushController, + RemoteFeatureFlagController: this.remoteFeatureFlagController, ...resetOnRestartStore, }); @@ -2573,6 +2625,7 @@ export default class MetamaskController extends EventEmitter { QueuedRequestController: this.queuedRequestController, NotificationServicesPushController: this.notificationServicesPushController, + RemoteFeatureFlagController: this.remoteFeatureFlagController, ...resetOnRestartStore, }, controllerMessenger: this.controllerMessenger, @@ -7359,6 +7412,17 @@ export default class MetamaskController extends EventEmitter { }; } + _getConfigForRemoteFeatureFlagRequest() { + const distribution = + buildTypeMappingForRemoteFeatureFlag[process.env.METAMASK_BUILD_TYPE] || + 'main'; + const environment = + environmentMappingForRemoteFeatureFlag[ + process.env.METAMASK_ENVIRONMENT + ] || 'dev'; + return { distribution, environment }; + } + #checkTokenListPolling(currentState, previousState) { const previousEnabled = this.#isTokenListPollingRequired(previousState); const newEnabled = this.#isTokenListPollingRequired(currentState); diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index a596277154eb..ccd20de0ff25 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -39,6 +39,7 @@ import { flushPromises } from '../../test/lib/timer-helpers'; import { ETH_EOA_METHODS } from '../../shared/constants/eth-methods'; import { createMockInternalAccount } from '../../test/jest/mocks'; import { mockNetworkState } from '../../test/stub/networks'; +import { ENVIRONMENT } from '../../development/build/constants'; import { SECOND } from '../../shared/constants/time'; import { BalancesController as MultichainBalancesController, @@ -2601,6 +2602,109 @@ describe('MetaMaskController', () => { ); }); }); + + describe('RemoteFeatureFlagController', () => { + let localMetamaskController; + let mockClientConfigApiService; + + beforeEach(() => { + mockClientConfigApiService = { + fetchRemoteFeatureFlags: jest + .fn() + .mockResolvedValue({ cachedData: [] }), + }; + + localMetamaskController = new MetaMaskController({ + showUserConfirmation: noop, + initLangCode: 'en_US', + platform: { + showTransactionNotification: () => undefined, + getVersion: () => 'foo', + }, + browser: browserPolyfillMock, + infuraProjectId: 'foo', + isFirstMetaMaskControllerSetup: true, + initState: { + ...cloneDeep(firstTimeState), + PreferencesController: { + useExternalServices: false, + }, + }, + }); + }); + + it('should initialize RemoteFeatureFlagController in disabled state when useExternalServices is false', async () => { + const { remoteFeatureFlagController, preferencesController } = + localMetamaskController; + // Check preferences state + expect(preferencesController.state.useExternalServices).toBe(false); + + // Since disabled is true, getRemoteFeatureFlags should return empty array + const flags = await remoteFeatureFlagController.getRemoteFeatureFlags(); + expect(flags).toStrictEqual([]); + + // Check controller state + expect(remoteFeatureFlagController.state).toStrictEqual({ + remoteFeatureFlags: [], + cacheTimestamp: 0, + }); + }); + + it('should enable/disable feature flag fetching based on useExternalServices in PreferenceController', async () => { + const { remoteFeatureFlagController } = localMetamaskController; + + // Initially disabled + let flags = await remoteFeatureFlagController.getRemoteFeatureFlags(); + expect(flags).toStrictEqual([]); + + // Enable external services + await simulatePreferencesChange({ + useExternalServices: true, + }); + + // Mock the service to return some flags + const mockFlags = [{ 'test-flag': true }]; + jest + .spyOn(remoteFeatureFlagController, 'getRemoteFeatureFlags') + .mockResolvedValueOnce(mockFlags); + + // Should now fetch flags + flags = await remoteFeatureFlagController.getRemoteFeatureFlags(); + expect(flags).toStrictEqual(mockFlags); + + // Disable external services + await simulatePreferencesChange({ + useExternalServices: false, + }); + + // Should return empty array again + flags = await remoteFeatureFlagController.getRemoteFeatureFlags(); + expect(flags).toStrictEqual([]); + }); + }); + + describe('_getConfigForRemoteFeatureFlagRequest', () => { + it('should return config in mapping', async () => { + const result = + await metamaskController._getConfigForRemoteFeatureFlagRequest(); + expect(result).toStrictEqual({ + distribution: 'main', + environment: 'dev', + }); + }); + + it('should return config when not matching default mapping', async () => { + process.env.METAMASK_BUILD_TYPE = 'beta'; + process.env.METAMASK_ENVIRONMENT = ENVIRONMENT.RELEASE_CANDIDATE; + + const result = + await metamaskController._getConfigForRemoteFeatureFlagRequest(); + expect(result).toStrictEqual({ + distribution: 'main', + environment: 'rc', + }); + }); + }); }); describe('onFeatureFlagResponseReceived', () => { diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index 47227aeef932..3e9af5026e0c 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -2338,6 +2338,16 @@ "semver": true } }, + "@metamask/remote-feature-flag-controller": { + "globals": { + "console.error": true + }, + "packages": { + "@metamask/base-controller": true, + "@metamask/utils": true, + "cockatiel": true + } + }, "@metamask/rpc-errors": { "packages": { "@metamask/rpc-errors>fast-safe-stringify": true, diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index 47227aeef932..3e9af5026e0c 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -2338,6 +2338,16 @@ "semver": true } }, + "@metamask/remote-feature-flag-controller": { + "globals": { + "console.error": true + }, + "packages": { + "@metamask/base-controller": true, + "@metamask/utils": true, + "cockatiel": true + } + }, "@metamask/rpc-errors": { "packages": { "@metamask/rpc-errors>fast-safe-stringify": true, diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index 47227aeef932..3e9af5026e0c 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -2338,6 +2338,16 @@ "semver": true } }, + "@metamask/remote-feature-flag-controller": { + "globals": { + "console.error": true + }, + "packages": { + "@metamask/base-controller": true, + "@metamask/utils": true, + "cockatiel": true + } + }, "@metamask/rpc-errors": { "packages": { "@metamask/rpc-errors>fast-safe-stringify": true, diff --git a/lavamoat/browserify/mmi/policy.json b/lavamoat/browserify/mmi/policy.json index 89eedc822794..5e93897b666d 100644 --- a/lavamoat/browserify/mmi/policy.json +++ b/lavamoat/browserify/mmi/policy.json @@ -2430,6 +2430,16 @@ "semver": true } }, + "@metamask/remote-feature-flag-controller": { + "globals": { + "console.error": true + }, + "packages": { + "@metamask/base-controller": true, + "@metamask/utils": true, + "cockatiel": true + } + }, "@metamask/rpc-errors": { "packages": { "@metamask/rpc-errors>fast-safe-stringify": true, diff --git a/lavamoat/build-system/policy.json b/lavamoat/build-system/policy.json index 5338922720ef..06d6042f6433 100644 --- a/lavamoat/build-system/policy.json +++ b/lavamoat/build-system/policy.json @@ -2040,7 +2040,8 @@ "chokidar>normalize-path": true, "chokidar>readdirp": true, "del>is-glob": true, - "eslint>glob-parent": true + "eslint>glob-parent": true, + "tsx>fsevents": true } }, "chokidar>anymatch": { @@ -8838,6 +8839,13 @@ "typescript": true } }, + "tsx>fsevents": { + "globals": { + "console.assert": true, + "process.platform": true + }, + "native": true + }, "typescript": { "builtin": { "buffer.Buffer": true, diff --git a/package.json b/package.json index e7827cb6d92b..581783014b05 100644 --- a/package.json +++ b/package.json @@ -335,6 +335,7 @@ "@metamask/providers": "^18.2.0", "@metamask/queued-request-controller": "^7.0.1", "@metamask/rate-limit-controller": "^6.0.0", + "@metamask/remote-feature-flag-controller": "file:../core/packages/remote-feature-flag-controller", "@metamask/rpc-errors": "^7.0.0", "@metamask/safe-event-emitter": "^3.1.1", "@metamask/scure-bip39": "^2.0.3", diff --git a/privacy-snapshot.json b/privacy-snapshot.json index 5620903a5c73..e7c5517d3a6e 100644 --- a/privacy-snapshot.json +++ b/privacy-snapshot.json @@ -16,6 +16,7 @@ "cdn.segment.io", "cdnjs.cloudflare.com", "chainid.network", + "client-config.api.cx.metamask.io", "client-side-detection.api.cx.metamask.io", "configuration.dev.metamask-institutional.io", "configuration.metamask-institutional.io", @@ -72,4 +73,4 @@ "unresponsive-rpc.url", "user-storage.api.cx.metamask.io", "www.4byte.directory" -] +] \ No newline at end of file diff --git a/test/e2e/fixture-builder.js b/test/e2e/fixture-builder.js index 844c4766db3e..8c68bb88495c 100644 --- a/test/e2e/fixture-builder.js +++ b/test/e2e/fixture-builder.js @@ -241,6 +241,12 @@ class FixtureBuilder { }); } + withUseBasicFunctionalityDisabled() { + return this.withPreferencesController({ + useExternalServices: false, + }); + } + withGasFeeController(data) { merge(this.fixture.data.GasFeeController, data); return this; diff --git a/test/e2e/mock-e2e.js b/test/e2e/mock-e2e.js index fc6b1ea4397a..0e3cb124a125 100644 --- a/test/e2e/mock-e2e.js +++ b/test/e2e/mock-e2e.js @@ -125,7 +125,6 @@ async function setupMocking( }); const mockedEndpoint = await testSpecificMock(server); - // Mocks below this line can be overridden by test-specific mocks // Account link @@ -738,6 +737,21 @@ async function setupMocking( }; }); + // remote feature flags + await server + .forGet('https://client-config.api.cx.metamask.io/v1/flags') + .withQuery({ + client: 'extension', + distribution: 'main', + environment: 'dev', + }) + .thenCallback(() => { + return { + statusCode: 200, + json: [{ feature1: true, feature2: false }], + }; + }); + /** * Returns an array of alphanumerically sorted hostnames that were requested * during the current test suite. diff --git a/test/e2e/tests/remote-feature-flag/mock-data.ts b/test/e2e/tests/remote-feature-flag/mock-data.ts new file mode 100644 index 000000000000..c997c504e215 --- /dev/null +++ b/test/e2e/tests/remote-feature-flag/mock-data.ts @@ -0,0 +1,3 @@ +export const MOCK_REMOTE_FEATURE_FLAGS_RESPONSE = [ + { feature1: true, feature2: false }, +]; diff --git a/test/e2e/tests/remote-feature-flag/remote-feature-flag.spec.ts b/test/e2e/tests/remote-feature-flag/remote-feature-flag.spec.ts new file mode 100644 index 000000000000..9fe319c655c9 --- /dev/null +++ b/test/e2e/tests/remote-feature-flag/remote-feature-flag.spec.ts @@ -0,0 +1,42 @@ +import { strict as assert } from 'assert'; +import { Suite } from 'mocha'; +import { getCleanAppState, withFixtures } from '../../helpers'; +import FixtureBuilder from '../../fixture-builder'; +import { TestSuiteArguments } from '../confirmations/transactions/shared'; +import { loginWithBalanceValidation } from '../../page-objects/flows/login.flow'; +import { MOCK_REMOTE_FEATURE_FLAGS_RESPONSE } from './mock-data'; + +describe('Remote feature flag', function (this: Suite) { + it('should be fetched when basic functionality toggle is on', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + title: this.test?.fullTitle(), + }, + async ({ driver }: TestSuiteArguments) => { + await loginWithBalanceValidation(driver); + const uiState = await getCleanAppState(driver); + assert.deepStrictEqual( + uiState.metamask.remoteFeatureFlags, + MOCK_REMOTE_FEATURE_FLAGS_RESPONSE, + ); + }, + ); + }); + + it('should not be fetched when basic functionality toggle is off', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder() + .withUseBasicFunctionalityDisabled() + .build(), + title: this.test?.fullTitle(), + }, + async ({ driver }: TestSuiteArguments) => { + await loginWithBalanceValidation(driver); + const uiState = await getCleanAppState(driver); + assert.deepStrictEqual(uiState.metamask.remoteFeatureFlags, []); + }, + ); + }); +}); diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 2baaa1a0c931..4711ad324a20 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -2911,6 +2911,10 @@ export function getMetaMetricsDataDeletionStatus(state) { return state.metamask.metaMetricsDataDeletionStatus; } +export function getRemoteFeatureFlags(state) { + return state.metamask.remoteFeatureFlags; +} + /** * To get all installed snaps with proper metadata * diff --git a/yarn.lock b/yarn.lock index 91cc4445c553..b3d5570f16f5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6139,6 +6139,17 @@ __metadata: languageName: node linkType: hard +"@metamask/remote-feature-flag-controller@file:../core/packages/remote-feature-flag-controller::locator=metamask-crx%40workspace%3A.": + version: 0.0.0 + resolution: "@metamask/remote-feature-flag-controller@file:../core/packages/remote-feature-flag-controller#../core/packages/remote-feature-flag-controller::hash=fceb11&locator=metamask-crx%40workspace%3A." + dependencies: + "@metamask/base-controller": "npm:^7.0.2" + "@metamask/utils": "npm:^10.0.0" + cockatiel: "npm:^3.1.2" + checksum: 10/4cdf15ef3c2aafc3c9d9d7a224d1af72c7c06535b5d1227d509ccef9d8c240211a97b7e021f957b630473937c1107097017b7c660456c1bde1f95055ff3524b6 + languageName: node + linkType: hard + "@metamask/rpc-errors@npm:^6.0.0, @metamask/rpc-errors@npm:^6.2.1, @metamask/rpc-errors@npm:^6.3.0, @metamask/rpc-errors@npm:^6.3.1": version: 6.4.0 resolution: "@metamask/rpc-errors@npm:6.4.0" @@ -26552,6 +26563,7 @@ __metadata: "@metamask/providers": "npm:^18.2.0" "@metamask/queued-request-controller": "npm:^7.0.1" "@metamask/rate-limit-controller": "npm:^6.0.0" + "@metamask/remote-feature-flag-controller": "file:../core/packages/remote-feature-flag-controller" "@metamask/rpc-errors": "npm:^7.0.0" "@metamask/safe-event-emitter": "npm:^3.1.1" "@metamask/scure-bip39": "npm:^2.0.3"