chore(runway): cherry-pick feat: implement remote feature flag contro…

…ller (#12510) - feat: implement remote feature flag controller (#12427)  ## **Description**  Introduction of `@metamask/remote-feature-flag-controller` library. Remote feature flag controller manages data flow, retry policy, and cache expiry. The controller consumer manages default values, data persistency, and data distribution. See [ADR](https://github.com/MetaMask/decisions/blob/b3094d47a568ac1e076a44fa704c2d29d1b59f35/decisions/wallet-platform/0001-remote-rollout-feature-flags.md) for a in-depth description ## Technical decisions ### Controller init on `Engine.ts` with feature flags fetching only on cold app starts. `@metamask/remote-feature-flag-controller` is only asked to fetch feature flags after its init in `Engine.ts`. Ensures feature flags are only fetched on cold app starts. ### Fallback values Default values are used when remote feature flags are undefined. The fallback mechanism is implemented by each feature flag selector `app/selectors/featureFlagsController/<featureFlagName>` In this PR we include `minimumAppVersion` selector, which manages the LD feature flag `mobile-minimum-versions` ### One selector per each feature flag [LD feature flags can be boolean, number, strings on JSON objects](https://docs.launchdarkly.com/sdk/concepts/flag-types#understanding-flag-types). We've decided to have each feature flag with its own selector A feature flag selector contains: - state selectors for each feature flag value. - business logic - defaults for when feature flags values are undefined. - TS types. - unit tests and mocked data. This architecture offers a clear separation between each feature flag and the logic behind it, allowing easier manipulation. Code owners are assigned to each feature flag. ### ## **Related issues** Fixes: https://github.com/MetaMask/mobile-planning/issues/2054 Fixes: https://github.com/MetaMask/mobile-planning/issues/1975 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings**  ### **Before**  ### **After**  ## **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. --------- Co-authored-by: Nico MASSART <[email protected]> Co-authored-by: tommasini <[email protected]> [a8c0783](a8c0783) Co-authored-by: João Loureiro <[email protected]> Co-authored-by: Nico MASSART <[email protected]> Co-authored-by: tommasini <[email protected]>
MetaMask · Dec 2, 2024 · 18aca38 · 18aca38
1 parent e958128
commit 18aca38
Show file tree

Hide file tree

Showing 33 changed files with 670 additions and 227 deletions.
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -7,19 +7,22 @@
 app/component-library/               @MetaMask/design-system-engineers
 
 # Platform Team
-.github/CODEOWNERS                   @MetaMask/mobile-platform
-patches/                             @MetaMask/mobile-platform
-app/core/Engine/Engine.ts            @MetaMask/mobile-platform
-app/core/Engine/Engine.test.ts       @MetaMask/mobile-platform
-app/core/Engine/index.ts             @MetaMask/mobile-platform
-app/core/Engine/types.ts             @MetaMask/mobile-platform
-app/core/Analytics/                  @MetaMask/mobile-platform
-app/util/metrics/                    @MetaMask/mobile-platform
-app/components/hooks/useMetrics/     @MetaMask/mobile-platform
-app/store/migrations/                @MetaMask/mobile-platform
-bitrise.yml                          @MetaMask/mobile-platform
-yarn.lock                            @MetaMask/mobile-platform
-ios/Podfile.lock                     @MetaMask/mobile-platform
+.github/CODEOWNERS                                          @MetaMask/mobile-platform
+patches/                                                    @MetaMask/mobile-platform
+app/core/Engine/Engine.ts                                   @MetaMask/mobile-platform
+app/core/Engine/Engine.test.ts                              @MetaMask/mobile-platform
+app/core/Engine/index.ts                                    @MetaMask/mobile-platform
+app/core/Engine/types.ts                                    @MetaMask/mobile-platform
+app/core/Engine/controllers/RemoteFeatureFlagController/    @MetaMask/mobile-platform
+app/core/Analytics/                                         @MetaMask/mobile-platform
+app/util/metrics/                                           @MetaMask/mobile-platform
+app/components/hooks/useMetrics/                            @MetaMask/mobile-platform
+app/selectors/featureFlagController/*                       @MetaMask/mobile-platform
+app/selectors/featureFlagController/minimumAppVersion/      @MetaMask/mobile-platform
+app/store/migrations/                                       @MetaMask/mobile-platform
+bitrise.yml                                                 @MetaMask/mobile-platform
+yarn.lock                                                   @MetaMask/mobile-platform
+ios/Podfile.lock                                            @MetaMask/mobile-platform
 
 # Ramps Team
 app/components/UI/Ramp/              @MetaMask/ramp
@@ -53,10 +56,10 @@ app/components/UI/Swaps              @MetaMask/swaps-engineers
 app/components/Views/Notifications @MetaMask/notifications
 app/components/Views/Settings/NotificationsSettings @MetaMask/notifications
 app/components/UI/Notifications @MetaMask/notifications
-app/reducers/notification  @MetaMask/notifications 
-app/actions/notification @MetaMask/notifications 
-app/selectors/notification @MetaMask/notifications 
-app/util/notifications  @MetaMask/notifications 
+app/reducers/notification  @MetaMask/notifications
+app/actions/notification @MetaMask/notifications
+app/selectors/notification @MetaMask/notifications
+app/util/notifications  @MetaMask/notifications
 app/store/util/notifications @MetaMask/notifications
 
 # LavaMoat Team

diff --git a/app/components/hooks/MinimumVersions/useMinimumVersions.test.ts b/app/components/hooks/MinimumVersions/useMinimumVersions.test.ts
@@ -35,12 +35,21 @@ describe('useMinimumVersions', () => {
     jest.clearAllMocks();
     (useNavigation as jest.Mock).mockReturnValue(mockNavigation);
   });
-
   it('requires update only if automaticSecurityChecksEnabled', () => {
     (useSelector as jest.Mock).mockImplementation(() => ({
       security: { automaticSecurityChecksEnabled: false },
-      featureFlags: {
-        featureFlags: { mobileMinimumVersions: { appMinimumBuild: 100 } },
+      engine: {
+        backgroundState: {
+          RemoteFeatureFlagController: {
+            remoteFeatureFlags: {
+              mobileMinimumVersions: {
+                appMinimumBuild: 100,
+                appleMinimumOS: 100,
+                androidMinimumAPIVersion: 100,
+              },
+            },
+          },
+        },
       },
     }));
 
@@ -54,8 +63,18 @@ describe('useMinimumVersions', () => {
   it('requires update only if currentBuildNumber is lower than appMinimumBuild', () => {
     (useSelector as jest.Mock).mockImplementation(() => ({
       security: { automaticSecurityChecksEnabled: true },
-      featureFlags: {
-        featureFlags: { mobileMinimumVersions: { appMinimumBuild: 100 } },
+      engine: {
+        backgroundState: {
+          RemoteFeatureFlagController: {
+            remoteFeatureFlags: {
+              mobileMinimumVersions: {
+                appMinimumBuild: 100,
+                appleMinimumOS: 100,
+                androidMinimumAPIVersion: 100,
+              },
+            },
+          },
+        },
       },
     }));
 

diff --git a/app/components/hooks/MinimumVersions/useMinimumVersions.tsx b/app/components/hooks/MinimumVersions/useMinimumVersions.tsx
@@ -4,22 +4,20 @@ import { createUpdateNeededNavDetails } from '../../UI/UpdateNeeded/UpdateNeeded
 import { useSelector } from 'react-redux';
 import { useNavigation } from '@react-navigation/native';
 import { InteractionManager } from 'react-native';
-import { FeatureFlagsState } from '../../../core/redux/slices/featureFlags';
-import { SecurityState } from '../../../../app/reducers/security';
-import { RootState } from '../../../../app/reducers';
+import { SecurityState } from '../../../reducers/security';
+import { RootState } from '../../../reducers';
+import { selectAppMinimumBuild } from '../../../selectors/featureFlagController/minimumAppVersion';
 
 const useMinimumVersions = () => {
   const { automaticSecurityChecksEnabled }: SecurityState = useSelector(
     (state: RootState) => state.security,
   );
-  const { featureFlags }: FeatureFlagsState = useSelector(
-    (state: RootState) => state.featureFlags,
-  );
+
+  const appMinimumBuild = useSelector((state: RootState) => selectAppMinimumBuild(state));
   const currentBuildNumber = Number(getBuildNumber());
   const navigation = useNavigation();
   const shouldTriggerUpdateFlow =
-    automaticSecurityChecksEnabled &&
-    featureFlags?.mobileMinimumVersions?.appMinimumBuild > currentBuildNumber;
+    automaticSecurityChecksEnabled && appMinimumBuild > currentBuildNumber;
 
   useEffect(() => {
     if (shouldTriggerUpdateFlow) {

diff --git a/app/core/Engine/Engine.test.ts b/app/core/Engine/Engine.test.ts
@@ -21,7 +21,9 @@ jest.mock('../../store', () => ({
 jest.mock('../../selectors/smartTransactionsController', () => ({
   selectShouldUseSmartTransaction: jest.fn().mockReturnValue(false),
 }));
-
+jest.mock('../../selectors/settings', () => ({
+  selectBasicFunctionalityEnabled: jest.fn().mockReturnValue(true),
+}));
 describe('Engine', () => {
   it('should expose an API', () => {
     const engine = Engine.init({});
@@ -37,6 +39,7 @@ describe('Engine', () => {
     expect(engine.context).toHaveProperty('NetworkController');
     expect(engine.context).toHaveProperty('PhishingController');
     expect(engine.context).toHaveProperty('PreferencesController');
+    expect(engine.context).toHaveProperty('RemoteFeatureFlagController');
     expect(engine.context).toHaveProperty('SignatureController');
     expect(engine.context).toHaveProperty('TokenBalancesController');
     expect(engine.context).toHaveProperty('TokenRatesController');

diff --git a/app/core/Engine/Engine.ts b/app/core/Engine/Engine.ts
@@ -1,6 +1,7 @@
 /* eslint-disable @typescript-eslint/no-shadow */
 import Crypto from 'react-native-quick-crypto';
 import { scrypt } from 'react-native-fast-crypto';
+
 import {
   AccountTrackerController,
   AssetsContractController,
@@ -153,6 +154,7 @@ import {
 } from './controllers/accounts/constants';
 import { AccountsControllerMessenger } from '@metamask/accounts-controller';
 import { createAccountsController } from './controllers/accounts/utils';
+import { createRemoteFeatureFlagController } from './controllers/RemoteFeatureFlagController';
 import { captureException } from '@sentry/react-native';
 import { lowerCase } from 'lodash';
 import {
@@ -161,6 +163,7 @@ import {
 } from '../../core/redux/slices/inpageProvider';
 import SmartTransactionsController from '@metamask/smart-transactions-controller';
 import { getAllowedSmartTransactionsChainIds } from '../../../app/constants/smartTransactions';
+import { selectBasicFunctionalityEnabled } from '../../selectors/settings';
 import { selectShouldUseSmartTransaction } from '../../selectors/smartTransactionsController';
 import { selectSwapsChainFeatureFlags } from '../../reducers/swaps';
 import { SmartTransactionStatuses, ClientId } from '@metamask/smart-transactions-controller/dist/types';
@@ -264,6 +267,9 @@ export class Engine {
   ) {
     this.controllerMessenger = new ExtendedControllerMessenger();
 
+    const isBasicFunctionalityToggleEnabled = () =>
+     selectBasicFunctionalityEnabled(store.getState());
+
     const approvalController = new ApprovalController({
       messenger: this.controllerMessenger.getRestricted({
         name: 'ApprovalController',
@@ -476,6 +482,16 @@ export class Engine {
         'https://gas.api.cx.metamask.io/networks/<chain_id>/suggestedGasFees',
     });
 
+    const remoteFeatureFlagController = createRemoteFeatureFlagController({
+      state: initialState.RemoteFeatureFlagController,
+      messenger: this.controllerMessenger.getRestricted({
+        name: 'RemoteFeatureFlagController',
+        allowedActions: [],
+        allowedEvents: [],
+      }),
+      disabled: !isBasicFunctionalityToggleEnabled(),
+    });
+
     const phishingController = new PhishingController({
       messenger: this.controllerMessenger.getRestricted({
         name: 'PhishingController',
@@ -924,8 +940,7 @@ export class Engine {
       encryptor,
       getMnemonic: getPrimaryKeyringMnemonic.bind(this),
       getFeatureFlags: () => ({
-        disableSnaps:
-          store.getState().settings.basicFunctionalityEnabled === false,
+        disableSnaps: !isBasicFunctionalityToggleEnabled(),
       }),
     });
 
@@ -1126,7 +1141,7 @@ export class Engine {
 
           return Boolean(
             hasProperty(showIncomingTransactions, currentChainId) &&
-              showIncomingTransactions?.[currentHexChainId],
+            showIncomingTransactions?.[currentHexChainId],
           );
         },
         updateTransactions: true,
@@ -1383,6 +1398,7 @@ export class Engine {
       GasFeeController: gasFeeController,
       ApprovalController: approvalController,
       PermissionController: permissionController,
+      RemoteFeatureFlagController: remoteFeatureFlagController,
       SelectedNetworkController: selectedNetworkController,
       SignatureController: new SignatureController({
         messenger: this.controllerMessenger.getRestricted({
@@ -1484,7 +1500,7 @@ export class Engine {
       (state: NetworkState) => {
         if (
           state.networksMetadata[state.selectedNetworkClientId].status ===
-            NetworkStatus.Available &&
+          NetworkStatus.Available &&
           networkController.getNetworkClientById(
             networkController?.state.selectedNetworkClientId,
           ).configuration.chainId !== currentChainId
@@ -1509,10 +1525,9 @@ export class Engine {
         } catch (error) {
           console.error(
             error,
-            `Network ID not changed, current chainId: ${
-              networkController.getNetworkClientById(
-                networkController?.state.selectedNetworkClientId,
-              ).configuration.chainId
+            `Network ID not changed, current chainId: ${networkController.getNetworkClientById(
+              networkController?.state.selectedNetworkClientId,
+            ).configuration.chainId
             }`,
           );
         }
@@ -1714,7 +1729,7 @@ export class Engine {
       const decimalsToShow = (currentCurrency === 'usd' && 2) || undefined;
       if (
         accountsByChainId?.[toHexadecimal(chainId)]?.[
-          selectSelectedInternalAccountChecksummedAddress
+        selectSelectedInternalAccountChecksummedAddress
         ]
       ) {
         const balanceBN = hexToBN(
@@ -1751,7 +1766,7 @@ export class Engine {
 
         const tokenBalances =
           allTokenBalances?.[selectedInternalAccount.address as Hex]?.[
-            chainId
+          chainId
           ] ?? {};
         tokens.forEach(
           (item: { address: string; balance?: string; decimals: number }) => {
@@ -1762,9 +1777,9 @@ export class Engine {
               item.balance ||
               (item.address in tokenBalances
                 ? renderFromTokenMinimalUnit(
-                    tokenBalances[item.address as Hex],
-                    item.decimals,
-                  )
+                  tokenBalances[item.address as Hex],
+                  item.decimals,
+                )
                 : undefined);
             const tokenBalanceFiat = balanceToFiatNumber(
               // TODO: Fix this by handling or eliminating the undefined case
@@ -2057,6 +2072,7 @@ export default {
       NetworkController,
       PreferencesController,
       PhishingController,
+      RemoteFeatureFlagController,
       PPOMController,
       TokenBalancesController,
       TokenRatesController,
@@ -2102,6 +2118,7 @@ export default {
       KeyringController,
       NetworkController,
       PhishingController,
+      RemoteFeatureFlagController,
       PPOMController,
       PreferencesController,
       TokenBalancesController,

diff --git a/app/core/Engine/controllers/RemoteFeatureFlagController/index.ts b/app/core/Engine/controllers/RemoteFeatureFlagController/index.ts
@@ -0,0 +1,2 @@
+export { createRemoteFeatureFlagController } from './utils';
+
diff --git a/app/core/Engine/controllers/RemoteFeatureFlagController/types.ts b/app/core/Engine/controllers/RemoteFeatureFlagController/types.ts
@@ -0,0 +1,8 @@
+import { RemoteFeatureFlagControllerMessenger, RemoteFeatureFlagControllerState } from '@metamask/remote-feature-flag-controller';
+
+export interface RemoteFeatureFlagInitParamTypes {
+  state?: RemoteFeatureFlagControllerState;
+  messenger: RemoteFeatureFlagControllerMessenger,
+  disabled: boolean
+}
+
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		export { createRemoteFeatureFlagController } from './utils';