Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(27256): implement remote feature flag feature #28684

Merged
merged 18 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* This function is called when MM is during internal process.
* This function is called during initialization.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just left a small nit

* 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 @@ -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": "^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
Loading