Skip to content

Commit

Permalink
feat: Enable feature Flag Values with Scope Based on "threshold" (#12627
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**

Remote Feature Flag Controller v1.3.0 requires `metaMetricsId` string
value for its initialization.
`metaMetricsId` is used to generate a threshold value for Scope based
feature flags.

On Mobile, `metaMetricsId` is managed by the MetaMetrics class, using
its method `getMetaMetricsId`.
Because this method returns a promise, we need to handle it outside
Engine constructor.

The route we opted is to await for the Promise to resolve on
`EngineService.start` function, then send it to `Engine.init` as a
parameter, which will pass it the into Engine constructor.

## **Related issues**

Fixes: MetaMask/mobile-planning#2065

## **Manual testing steps**

1. Go to this page...
2.
3.

## **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 Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/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.
  • Loading branch information
joaoloureirop authored Jan 23, 2025
1 parent c40c8fd commit 15cd391
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 38 deletions.
5 changes: 4 additions & 1 deletion app/core/Engine/Engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ export class Engine {
constructor(
initialState: Partial<EngineState> = {},
initialKeyringState?: KeyringControllerState | null,
metaMetricsId?: string,
) {
logEngineCreation(initialState, initialKeyringState);

Expand Down Expand Up @@ -514,6 +515,7 @@ export class Engine {
allowedEvents: [],
}),
disabled: !isBasicFunctionalityToggleEnabled(),
getMetaMetricsId: () => metaMetricsId ?? '',
});

const phishingController = new PhishingController({
Expand Down Expand Up @@ -2166,8 +2168,9 @@ export default {
init(
state: Partial<EngineState> | undefined,
keyringState: KeyringControllerState | null = null,
metaMetricsId?: string,
) {
instance = Engine.instance || new Engine(state, keyringState);
instance = Engine.instance || new Engine(state, keyringState, metaMetricsId);
Object.freeze(instance);
return instance;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@ export interface RemoteFeatureFlagInitParamTypes {
state?: RemoteFeatureFlagControllerState;
messenger: RemoteFeatureFlagControllerMessenger;
disabled: boolean;
getMetaMetricsId: () => string;
}

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
RemoteFeatureFlagControllerMessenger,
} from '@metamask/remote-feature-flag-controller';
import { createRemoteFeatureFlagController } from './utils';
import { v4 as uuidv4 } from 'uuid';

describe('RemoteFeatureFlagController utils', () => {
let messenger: RemoteFeatureFlagControllerMessenger;
Expand All @@ -20,6 +21,7 @@ describe('RemoteFeatureFlagController utils', () => {
state: undefined,
messenger,
disabled: false,
getMetaMetricsId: () => uuidv4(),
});

expect(controller).toBeDefined();
Expand All @@ -43,6 +45,7 @@ describe('RemoteFeatureFlagController utils', () => {
state: initialState,
messenger,
disabled: false,
getMetaMetricsId: () => uuidv4(),
});

expect(controller.state).toStrictEqual(initialState);
Expand All @@ -58,6 +61,7 @@ describe('RemoteFeatureFlagController utils', () => {
state: undefined,
messenger,
disabled: false,
getMetaMetricsId: () => uuidv4(),
});

expect(spy).toHaveBeenCalled();
Expand All @@ -73,6 +77,7 @@ describe('RemoteFeatureFlagController utils', () => {
state: undefined,
messenger,
disabled: true,
getMetaMetricsId: () => uuidv4(),
});

expect(spy).not.toHaveBeenCalled();
Expand All @@ -88,6 +93,7 @@ describe('RemoteFeatureFlagController utils', () => {
state: initialState,
messenger,
disabled: false,
getMetaMetricsId: () => uuidv4(),
});

expect(controller.state).toStrictEqual({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@ export const createRemoteFeatureFlagController = ({
state,
messenger,
disabled,
getMetaMetricsId,
}: RemoteFeatureFlagInitParamTypes) => {
const remoteFeatureFlagController = new RemoteFeatureFlagController({
messenger,
state,
disabled,
getMetaMetricsId,
clientConfigApiService: new ClientConfigApiService({
fetch,
config: {
Expand Down
62 changes: 36 additions & 26 deletions app/core/EngineService/EngineService.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { waitFor } from '@testing-library/react-native';
import { EngineService } from './EngineService';
import ReduxService, { type ReduxStore } from '../redux';
import Engine from '../Engine';
Expand Down Expand Up @@ -119,37 +120,44 @@ describe('EngineService', () => {
engineService = new EngineService();
});

it('should have Engine initialized', () => {
it('should have Engine initialized', async () => {
engineService.start();
expect(Engine.context).toBeDefined();
await waitFor(() => {
expect(Engine.context).toBeDefined();
});
});

it('should log Engine initialization with state info', () => {
it('should log Engine initialization with state info', async () => {
engineService.start();
expect(Logger.log).toHaveBeenCalledWith(
'EngineService: Initializing Engine:',
{
hasState: true,
},
);
await waitFor(() => {
expect(Logger.log).toHaveBeenCalledWith(
'EngineService: Initializing Engine:',
{
hasState: true,
},
);
});

});

it('should log Engine initialization with empty state', () => {
it('should log Engine initialization with empty state', async () => {
jest.spyOn(ReduxService, 'store', 'get').mockReturnValue({
getState: () => ({ engine: { backgroundState: {} } }),
} as unknown as ReduxStore);

engineService.start();
expect(Logger.log).toHaveBeenCalledWith(
'EngineService: Initializing Engine:',
{
hasState: false,
},
);
await waitFor(() => {
expect(Logger.log).toHaveBeenCalledWith(
'EngineService: Initializing Engine:',
{
hasState: false,
},
);
});
});

it('should have recovered vault on redux store and log initialization', async () => {
engineService.start();
await engineService.start();
const { success } = await engineService.initializeVaultFromBackup();
expect(success).toBeTruthy();
expect(Engine.context.KeyringController.state.vault).toBeDefined();
Expand All @@ -161,19 +169,21 @@ describe('EngineService', () => {
);
});

it('should navigate to vault recovery if Engine fails to initialize', () => {
it('should navigate to vault recovery if Engine fails to initialize', async () => {
jest.spyOn(Engine, 'init').mockImplementation(() => {
throw new Error('Failed to initialize Engine');
});
engineService.start();
// Logs error to Sentry
expect(Logger.error).toHaveBeenCalledWith(
new Error('Failed to initialize Engine'),
'Failed to initialize Engine! Falling back to vault recovery.',
);
// Navigates to vault recovery
expect(NavigationService.navigation?.reset).toHaveBeenCalledWith({
routes: [{ name: Routes.VAULT_RECOVERY.RESTORE_WALLET }],
await waitFor(() => {
// Logs error to Sentry
expect(Logger.error).toHaveBeenCalledWith(
new Error('Failed to initialize Engine'),
'Failed to initialize Engine! Falling back to vault recovery.',
);
// Navigates to vault recovery
expect(NavigationService.navigation?.reset).toHaveBeenCalledWith({
routes: [{ name: Routes.VAULT_RECOVERY.RESTORE_WALLET }],
});
});
});
});
9 changes: 6 additions & 3 deletions app/core/EngineService/EngineService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import ReduxService from '../redux';
import NavigationService from '../NavigationService';
import Routes from '../../constants/navigation/Routes';
import { KeyringControllerState } from '@metamask/keyring-controller';
import { MetaMetrics } from '../Analytics';

const LOG_TAG = 'EngineService';

Expand Down Expand Up @@ -43,7 +44,7 @@ export class EngineService {
* - TypeError: undefined is not an object (evaluating 'TokenListController.tokenList')
* - V8: SES_UNHANDLED_REJECTION
*/
start = () => {
start = async () => {
const reduxState = ReduxService.store.getState();
trace({
name: TraceName.EngineInitialization,
Expand All @@ -57,7 +58,8 @@ export class EngineService {
hasState: Object.keys(state).length > 0,
});

Engine.init(state);
const metaMetricsId = await MetaMetrics.getInstance().getMetaMetricsId();
Engine.init(state, null, metaMetricsId);
this.updateControllers(Engine);
} catch (error) {
Logger.error(
Expand Down Expand Up @@ -257,7 +259,8 @@ export class EngineService {
hasState: Object.keys(state).length > 0,
});

const instance = Engine.init(state, newKeyringState);
const metaMetricsId = await MetaMetrics.getInstance().getMetaMetricsId();
const instance = Engine.init(state, newKeyringState, metaMetricsId);
if (instance) {
this.updateControllers(instance);
// this is a hack to give the engine time to reinitialize
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@
"@metamask/react-native-payments": "^2.0.0",
"@metamask/react-native-search-api": "1.0.1",
"@metamask/react-native-webview": "^14.0.4",
"@metamask/remote-feature-flag-controller": "^1.0.0",
"@metamask/remote-feature-flag-controller": "^1.3.0",
"@metamask/rpc-errors": "^7.0.2",
"@metamask/scure-bip39": "^2.1.0",
"@metamask/sdk-communication-layer": "0.29.0-wallet",
Expand Down
15 changes: 8 additions & 7 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4512,7 +4512,7 @@
single-call-balance-checker-abi "^1.0.0"
uuid "^8.3.2"

"@metamask/base-controller@^7.0.1", "@metamask/base-controller@^7.0.2", "@metamask/base-controller@^7.1.1":
"@metamask/base-controller@^7.0.1", "@metamask/base-controller@^7.0.2", "@metamask/base-controller@^7.1.0", "@metamask/base-controller@^7.1.1":
version "7.1.1"
resolved "https://registry.yarnpkg.com/@metamask/base-controller/-/base-controller-7.1.1.tgz#837216ee099563b2106202fa0ed376dc909dfbb9"
integrity sha512-4nbA6RL9y0SdHdn4MmMTREX6ISJL7OGHn0GXXszv0tp1fdjsn+SBs28uu1a9ceg1J7R/lO6JH7jAAz8zRtt8Nw==
Expand Down Expand Up @@ -5318,14 +5318,15 @@
escape-string-regexp "^4.0.0"
invariant "2.2.4"

"@metamask/remote-feature-flag-controller@^1.0.0":
version "1.0.0"
resolved "https://registry.yarnpkg.com/@metamask/remote-feature-flag-controller/-/remote-feature-flag-controller-1.0.0.tgz#048162eaa6fa34401cfbabfa0eb33f0255bb2945"
integrity sha512-jrjEQhW/RdHQ/GQbgXH97N6YqDUW7nGA40lEr0TUSIhJVVaHDX0gCiNmJZcQ89yLY4DZ0bisEwjrCu8LycYiQQ==
"@metamask/remote-feature-flag-controller@^1.3.0":
version "1.3.0"
resolved "https://registry.yarnpkg.com/@metamask/remote-feature-flag-controller/-/remote-feature-flag-controller-1.3.0.tgz#b83fc08c413b229b24046c84e2599d4dab2bafd8"
integrity sha512-h5DnnqbFxLsm8N98rrlVwUpQvvH03epb+1YhRMghIKa5WtjyoERV2wp0MuSH8l2Or4+ccx8eLv/X2bsnmtujGw==
dependencies:
"@metamask/base-controller" "^7.0.2"
"@metamask/utils" "^10.0.0"
"@metamask/base-controller" "^7.1.0"
"@metamask/utils" "^11.0.1"
cockatiel "^3.1.2"
uuid "^8.3.2"

"@metamask/[email protected]", "@metamask/rpc-errors@^6.2.1", "@metamask/rpc-errors@^7.0.0", "@metamask/rpc-errors@^7.0.1", "@metamask/rpc-errors@^7.0.2":
version "7.0.2"
Expand Down

0 comments on commit 15cd391

Please sign in to comment.