Skip to content

Commit

Permalink
feat(27256): implement remote feature flag feature (#28684)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

- Consume feature-flag-controller built in
#27254.
- And When Basic Functionality toggle is OFF, feature flag request
should not be made, not call getFeatureFlags, which can be aligned with
MetaMask/mobile-planning#1975

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

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

## **Related issues**

Fixes: #27256

## **Manual testing steps**

1.  Load the extension
2. Option 1: input `chrome.storage.local.get(console.log)` in dev tools
console, and you'll find in `data` => `RemoteFeatureFlagController` =>
contains 2 dataset, `cacheTimestamp` and `remoteFeatureFlags` with value
3. Option 2: go to settings => advanced, and click `download stage logs`
button, you'll find `remoteFeatureFlags` with cached value in your
state.
4. Option 3: Settings => about page, if you open console in dev tools,
there's a log after `Feature flag fetched successfully`, which will be
removed after we implement 1st feature flag in production

## **Screenshots/Recordings**

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

### **Before**

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

### **After**

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

## **Pre-merge author checklist**

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

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: Mark Stacey <[email protected]>
Co-authored-by: MetaMask Bot <[email protected]>
Co-authored-by: Dan J Miller <[email protected]>
  • Loading branch information
4 people authored Dec 3, 2024
1 parent af68fe5 commit 589d7e4
Show file tree
Hide file tree
Showing 26 changed files with 386 additions and 22 deletions.
19 changes: 18 additions & 1 deletion app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,6 @@ export function setupController(
//
// MetaMask Controller
//

controller = new MetamaskController({
infuraProjectId: process.env.INFURA_PROJECT_ID,
// User confirmation callbacks:
Expand Down Expand Up @@ -892,6 +891,8 @@ export function setupController(
controller.isClientOpen = true;
controller.setupTrustedCommunication(portStream, remotePort.sender);

initializeRemoteFeatureFlags();

if (processName === ENVIRONMENT_TYPE_POPUP) {
openPopupCount += 1;
finished(portStream, () => {
Expand Down Expand Up @@ -1094,6 +1095,22 @@ 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<void>} 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 {
let pendingApprovalCount =
Expand Down
4 changes: 4 additions & 0 deletions app/scripts/constants/sentry-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,10 @@ export const SENTRY_BACKGROUND_STATE = {
useTransactionSimulations: true,
enableMV3TimestampSave: true,
},
RemoteFeatureFlagController: {
remoteFeatureFlags: true,
cacheTimestamp: false,
},
NotificationServicesPushController: {
fcmToken: false,
},
Expand Down
66 changes: 66 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,13 @@ import {
} from '@metamask/controller-utils';

import { AccountsController } from '@metamask/accounts-controller';
import {
RemoteFeatureFlagController,
ClientConfigApiService,
ClientType,
DistributionType,
EnvironmentType,
} from '@metamask/remote-feature-flag-controller';

///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
import {
Expand Down Expand Up @@ -241,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 {
Expand Down Expand Up @@ -393,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]: EnvironmentType.Development,
[ENVIRONMENT.RELEASE_CANDIDATE]: EnvironmentType.ReleaseCandidate,
[ENVIRONMENT.PRODUCTION]: EnvironmentType.Production,
};

const buildTypeMappingForRemoteFeatureFlag = {
flask: DistributionType.Flask,
main: DistributionType.Main,
};

export default class MetamaskController extends EventEmitter {
/**
* @param {object} opts
Expand Down Expand Up @@ -2342,6 +2361,40 @@ 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.remoteFeatureFlagController.updateRemoteFeatureFlags();
} else if (!currUseExternalServices && prevUseExternalServices) {
this.remoteFeatureFlagController.disable();
}
}, this.preferencesController.state),
);

// Initialize RemoteFeatureFlagController
this.remoteFeatureFlagController = new RemoteFeatureFlagController({
messenger: this.controllerMessenger.getRestricted({
name: 'RemoteFeatureFlagController',
allowedActions: [],
allowedEvents: [],
}),
disabled: !this.preferencesController.state.useExternalServices,
clientConfigApiService: new ClientConfigApiService({
fetch: globalThis.fetch.bind(globalThis),
config: {
client: ClientType.Extension,
distribution:
this._getConfigForRemoteFeatureFlagRequest().distribution,
environment: this._getConfigForRemoteFeatureFlagRequest().environment,
},
}),
});

this.metamaskMiddleware = createMetamaskMiddleware({
static: {
eth_syncing: false,
Expand Down Expand Up @@ -2508,6 +2561,7 @@ export default class MetamaskController extends EventEmitter {
NotificationServicesController: this.notificationServicesController,
NotificationServicesPushController:
this.notificationServicesPushController,
RemoteFeatureFlagController: this.remoteFeatureFlagController,
...resetOnRestartStore,
});

Expand Down Expand Up @@ -2563,6 +2617,7 @@ export default class MetamaskController extends EventEmitter {
QueuedRequestController: this.queuedRequestController,
NotificationServicesPushController:
this.notificationServicesPushController,
RemoteFeatureFlagController: this.remoteFeatureFlagController,
...resetOnRestartStore,
},
controllerMessenger: this.controllerMessenger,
Expand Down Expand Up @@ -7364,6 +7419,17 @@ export default class MetamaskController extends EventEmitter {
};
}

_getConfigForRemoteFeatureFlagRequest() {
const distribution =
buildTypeMappingForRemoteFeatureFlag[process.env.METAMASK_BUILD_TYPE] ||
DistributionType.Main;
const environment =
environmentMappingForRemoteFeatureFlag[
process.env.METAMASK_ENVIRONMENT
] || EnvironmentType.Development;
return { distribution, environment };
}

#checkTokenListPolling(currentState, previousState) {
const previousEnabled = this.#isTokenListPollingRequired(previousState);
const newEnabled = this.#isTokenListPollingRequired(currentState);
Expand Down
126 changes: 126 additions & 0 deletions app/scripts/metamask-controller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,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,
Expand Down Expand Up @@ -2607,6 +2608,131 @@ describe('MetaMaskController', () => {
);
});
});

describe('RemoteFeatureFlagController', () => {
let localMetamaskController;

beforeEach(() => {
localMetamaskController = new MetaMaskController({
showUserConfirmation: noop,
encryptor: mockEncryptor,
initState: {
...cloneDeep(firstTimeState),
PreferencesController: {
useExternalServices: false,
},
},
initLangCode: 'en_US',
platform: {
showTransactionNotification: () => undefined,
getVersion: () => 'foo',
},
browser: browserPolyfillMock,
infuraProjectId: 'foo',
isFirstMetaMaskControllerSetup: true,
});
});

afterEach(() => {
jest.clearAllMocks();
});

it('should initialize RemoteFeatureFlagController in disabled state when useExternalServices is false', async () => {
const { remoteFeatureFlagController, preferencesController } =
localMetamaskController;

expect(preferencesController.state.useExternalServices).toBe(false);
expect(remoteFeatureFlagController.state).toStrictEqual({
remoteFeatureFlags: {},
cacheTimestamp: 0,
});
});

it('should disable feature flag fetching when useExternalServices is disabled', async () => {
const { remoteFeatureFlagController } = localMetamaskController;

// First enable external services
await simulatePreferencesChange({
useExternalServices: true,
});

// Then disable them
await simulatePreferencesChange({
useExternalServices: false,
});

expect(remoteFeatureFlagController.state).toStrictEqual({
remoteFeatureFlags: {},
cacheTimestamp: 0,
});
});

it('should handle errors during feature flag updates', async () => {
const { remoteFeatureFlagController } = localMetamaskController;
const mockError = new Error('Failed to fetch');

jest
.spyOn(remoteFeatureFlagController, 'updateRemoteFeatureFlags')
.mockRejectedValue(mockError);

await simulatePreferencesChange({
useExternalServices: true,
});

expect(remoteFeatureFlagController.state).toStrictEqual({
remoteFeatureFlags: {},
cacheTimestamp: 0,
});
});

it('should maintain feature flag state across preference toggles', async () => {
const { remoteFeatureFlagController } = localMetamaskController;
const mockFlags = { testFlag: true };

jest
.spyOn(remoteFeatureFlagController, 'updateRemoteFeatureFlags')
.mockResolvedValue(mockFlags);

// Enable external services
await simulatePreferencesChange({
useExternalServices: true,
});

// Disable external services
await simulatePreferencesChange({
useExternalServices: false,
});

// Verify state is cleared
expect(remoteFeatureFlagController.state).toStrictEqual({
remoteFeatureFlags: {},
cacheTimestamp: 0,
});
});
});

describe('_getConfigForRemoteFeatureFlagRequest', () => {
it('returns config in mapping', async () => {
const result =
await metamaskController._getConfigForRemoteFeatureFlagRequest();
expect(result).toStrictEqual({
distribution: 'main',
environment: 'dev',
});
});

it('returna 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', () => {
Expand Down
6 changes: 6 additions & 0 deletions lavamoat/browserify/beta/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -2338,6 +2338,12 @@
"semver": true
}
},
"@metamask/remote-feature-flag-controller": {
"packages": {
"@metamask/base-controller": true,
"cockatiel": true
}
},
"@metamask/rpc-errors": {
"packages": {
"@metamask/rpc-errors>fast-safe-stringify": true,
Expand Down
6 changes: 6 additions & 0 deletions lavamoat/browserify/flask/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -2338,6 +2338,12 @@
"semver": true
}
},
"@metamask/remote-feature-flag-controller": {
"packages": {
"@metamask/base-controller": true,
"cockatiel": true
}
},
"@metamask/rpc-errors": {
"packages": {
"@metamask/rpc-errors>fast-safe-stringify": true,
Expand Down
6 changes: 6 additions & 0 deletions lavamoat/browserify/main/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -2338,6 +2338,12 @@
"semver": true
}
},
"@metamask/remote-feature-flag-controller": {
"packages": {
"@metamask/base-controller": true,
"cockatiel": true
}
},
"@metamask/rpc-errors": {
"packages": {
"@metamask/rpc-errors>fast-safe-stringify": true,
Expand Down
6 changes: 6 additions & 0 deletions lavamoat/browserify/mmi/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -2430,6 +2430,12 @@
"semver": true
}
},
"@metamask/remote-feature-flag-controller": {
"packages": {
"@metamask/base-controller": true,
"cockatiel": true
}
},
"@metamask/rpc-errors": {
"packages": {
"@metamask/rpc-errors>fast-safe-stringify": true,
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,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": "^1.1.0",
"@metamask/rpc-errors": "^7.0.0",
"@metamask/safe-event-emitter": "^3.1.1",
"@metamask/scure-bip39": "^2.0.3",
Expand Down
1 change: 1 addition & 0 deletions privacy-snapshot.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 6 additions & 0 deletions test/e2e/fixture-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,12 @@ class FixtureBuilder {
});
}

withUseBasicFunctionalityDisabled() {
return this.withPreferencesController({
useExternalServices: false,
});
}

withGasFeeController(data) {
merge(this.fixture.data.GasFeeController, data);
return this;
Expand Down
Loading

0 comments on commit 589d7e4

Please sign in to comment.