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: implement remote feature flag controller #12427

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
00a3bd1
setup metro bundler symlink resolution
joaoloureirop Nov 22, 2024
b4f9cc8
yarn setup fixup
joaoloureirop Nov 22, 2024
cdd4a58
metro bundler: bypass lint errors
joaoloureirop Nov 22, 2024
1787680
init RemoteFeatureFlagController
joaoloureirop Nov 22, 2024
5299891
controller init state destruct
joaoloureirop Nov 26, 2024
6204914
add feature flag selectors
joaoloureirop Nov 26, 2024
6f8d208
ff(mobileMinimumVersion): set fallback values
joaoloureirop Nov 26, 2024
608f05b
remove feature flag saga / slice
joaoloureirop Nov 26, 2024
a8ec092
remote feature flags behing basic func toggle
joaoloureirop Nov 26, 2024
01df041
fix controller state change event
joaoloureirop Nov 26, 2024
c8c2af0
get env vars to init feature flag controllers
joaoloureirop Nov 26, 2024
ca33006
update min version fallback values
joaoloureirop Nov 26, 2024
27b32a3
remove controller messenger allowances
joaoloureirop Nov 26, 2024
d7aed0d
Revert "metro bundler: bypass lint errors"
joaoloureirop Nov 26, 2024
b80145d
remove unused import
joaoloureirop Nov 26, 2024
9a67929
fix lint errors
joaoloureirop Nov 26, 2024
934a13a
remove unneded semicolon
joaoloureirop Nov 26, 2024
faa6cb0
remove featureFlag initial root state
joaoloureirop Nov 26, 2024
20c3b42
Merge branch 'main' into feat/feature-flags
joaoloureirop Nov 27, 2024
d22d1cb
Engine: split Feature flag controller
joaoloureirop Nov 27, 2024
ba1802e
set RemoteFeatureFlagController & minappversion flag codeowners
joaoloureirop Nov 27, 2024
5e97075
minimumAppVersion featureflag: improve types
joaoloureirop Nov 27, 2024
c838b21
remove dead code
joaoloureirop Nov 27, 2024
426655e
add Feature flag controller logger
joaoloureirop Nov 27, 2024
ebc940e
Merge branch 'main' into feat/feature-flags
joaoloureirop Nov 27, 2024
8014b45
fix basic functionality toggle
joaoloureirop Nov 27, 2024
9a35568
Revert "yarn setup fixup"
joaoloureirop Nov 27, 2024
8c305c1
improve minimumAppVersion TS types
joaoloureirop Nov 27, 2024
c8495e5
fix unit tests
joaoloureirop Nov 27, 2024
d7d384c
migration: remove featureFlags property
joaoloureirop Nov 27, 2024
b784e89
codeowners: featureFlagController selector base files pattern
joaoloureirop Nov 27, 2024
38292f7
update useMinimumVersions.tsx import paths
joaoloureirop Nov 27, 2024
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
10 changes: 4 additions & 6 deletions app/components/hooks/MinimumVersions/useMinimumVersions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 { selectAppMinimumBuild } from '../../../../app/selectors/featureFlagController/minimumAppVersion';
joaoloureirop marked this conversation as resolved.
Show resolved Hide resolved

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));
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: not sure if this change makes the mock in the test obsolete and how this test doesn't need any changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, the tests for this hook need to be updated

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised the test is passing given the changes in the hook... so maybe rewrite the test in a more robust way. A change like this should make it fail otherwise it probably means it's not testing anything but the mocks...

const currentBuildNumber = Number(getBuildNumber());
const navigation = useNavigation();
const shouldTriggerUpdateFlow =
automaticSecurityChecksEnabled &&
featureFlags?.mobileMinimumVersions?.appMinimumBuild > currentBuildNumber;
automaticSecurityChecksEnabled && appMinimumBuild > currentBuildNumber;

useEffect(() => {
if (shouldTriggerUpdateFlow) {
Expand Down
79 changes: 69 additions & 10 deletions app/core/Engine.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
/* eslint-disable @typescript-eslint/no-shadow */
import Crypto from 'react-native-quick-crypto';
import { scrypt } from 'react-native-fast-crypto';
import {
RemoteFeatureFlagController,
RemoteFeatureFlagControllerState,
RemoteFeatureFlagControllerGetStateAction,
ClientConfigApiService,
ClientType,
DistributionType,
EnvironmentType,

} from '@metamask/remote-feature-flag-controller';

import {
AccountTrackerController,
AccountTrackerControllerState,
Expand Down Expand Up @@ -270,6 +281,7 @@ import { trace } from '../util/trace';
import { MetricsEventBuilder } from './Analytics/MetricsEventBuilder';
import { JsonMap } from './Analytics/MetaMetrics.types';
import { isPooledStakingFeatureEnabled } from '../components/UI/Stake/constants';
import { RemoteFeatureFlagControllerActions } from '@metamask/remote-feature-flag-controller/dist/remote-feature-flag-controller.cjs';

const NON_EMPTY = 'NON_EMPTY';

Expand Down Expand Up @@ -309,6 +321,7 @@ type GlobalActions =
| NetworkControllerActions
| PermissionControllerActions
| SignatureControllerActions
| RemoteFeatureFlagControllerGetStateAction
| LoggingControllerActions
///: BEGIN:ONLY_INCLUDE_IF(preinstalled-snaps,external-snaps)
| SnapsGlobalActions
Expand Down Expand Up @@ -370,6 +383,7 @@ export interface EngineState {
NetworkController: NetworkState;
PreferencesController: PreferencesState;
PhishingController: PhishingControllerState;
RemoteFeatureFlagController: RemoteFeatureFlagControllerState;
TokenBalancesController: TokenBalancesControllerState;
TokenRatesController: TokenRatesControllerState;
TransactionController: TransactionControllerState;
Expand Down Expand Up @@ -420,6 +434,7 @@ interface Controllers {
PhishingController: PhishingController;
PreferencesController: PreferencesController;
PPOMController: PPOMController;
RemoteFeatureFlagController: RemoteFeatureFlagController;
TokenBalancesController: TokenBalancesController;
TokenListController: TokenListController;
TokenDetectionController: TokenDetectionController;
Expand Down Expand Up @@ -744,6 +759,48 @@ export class Engine {
'https://gas.api.cx.metamask.io/networks/<chain_id>/suggestedGasFees',
});

const getFeatureFlagAppEnvironment = () => {
const env = process.env.METAMASK_ENVIRONMENT;
switch(env) {
case 'local': return EnvironmentType.Development;
case 'pre-release': return EnvironmentType.ReleaseCandidate;
case 'production': return EnvironmentType.Production;
default: return EnvironmentType.Development;
}
};
const getFeatureFlagAppDistribution = () => {
const dist = process.env.METAMASK_BUILD_TYPE;
switch(dist) {
case 'main': return DistributionType.Main;
case 'flask': return DistributionType.Flask;
default: return DistributionType.Main;
}

};

const featureFlagController = new RemoteFeatureFlagController({
messenger: this.controllerMessenger.getRestricted({
name: 'RemoteFeatureFlagController',
allowedActions: ['PreferencesController:getState'],
allowedEvents: ['PreferencesController:stateChange'],
joaoloureirop marked this conversation as resolved.
Show resolved Hide resolved
}),
state: {
...initialState.RemoteFeatureFlagController
},
disabled:
store.getState().settings.basicFunctionalityEnabled === false,
clientConfigApiService: new ClientConfigApiService({
fetch: fetchFunction,
config: {
client: ClientType.Mobile,
environment: getFeatureFlagAppEnvironment(),
distribution: getFeatureFlagAppDistribution(),
},
}),
});

featureFlagController.getRemoteFeatureFlags();

const phishingController = new PhishingController({
messenger: this.controllerMessenger.getRestricted({
name: 'PhishingController',
Expand Down Expand Up @@ -1401,7 +1458,7 @@ export class Engine {

return Boolean(
hasProperty(showIncomingTransactions, currentChainId) &&
showIncomingTransactions?.[currentHexChainId],
showIncomingTransactions?.[currentHexChainId],
);
},
updateTransactions: true,
Expand Down Expand Up @@ -1648,6 +1705,7 @@ export class Engine {
gasFeeController,
approvalController,
permissionController,
featureFlagController,
selectedNetworkController,
new SignatureController({
messenger: this.controllerMessenger.getRestricted({
Expand Down Expand Up @@ -1762,7 +1820,7 @@ export class Engine {
(state: NetworkState) => {
if (
state.networksMetadata[state.selectedNetworkClientId].status ===
NetworkStatus.Available &&
NetworkStatus.Available &&
networkController.getNetworkClientById(
networkController?.state.selectedNetworkClientId,
).configuration.chainId !== currentChainId
Expand All @@ -1787,10 +1845,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
}`,
);
}
Expand Down Expand Up @@ -1993,7 +2050,7 @@ export class Engine {
const decimalsToShow = (currentCurrency === 'usd' && 2) || undefined;
if (
accountsByChainId?.[toHexadecimal(chainId)]?.[
selectSelectedInternalAccountChecksummedAddress
selectSelectedInternalAccountChecksummedAddress
]
) {
const balanceBN = hexToBN(
Expand Down Expand Up @@ -2036,9 +2093,9 @@ export class Engine {
item.balance ||
(item.address in tokenBalances
? renderFromTokenMinimalUnit(
tokenBalances[item.address],
item.decimals,
)
tokenBalances[item.address],
item.decimals,
)
: undefined);
const tokenBalanceFiat = balanceToFiatNumber(
// TODO: Fix this by handling or eliminating the undefined case
Expand Down Expand Up @@ -2330,6 +2387,7 @@ export default {
NetworkController,
PreferencesController,
PhishingController,
RemoteFeatureFlagController,
PPOMController,
TokenBalancesController,
TokenRatesController,
Expand Down Expand Up @@ -2375,6 +2433,7 @@ export default {
KeyringController,
NetworkController,
PhishingController,
RemoteFeatureFlagController,
PPOMController,
PreferencesController,
TokenBalancesController,
Expand Down
4 changes: 4 additions & 0 deletions app/core/EngineService/EngineService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ class EngineService {
name: 'PreferencesController',
key: `${engine.context.PreferencesController.name}:stateChange`,
},
{
name: 'RemoteFeatureFlagController',
key: `${engine.context.RemoteFeatureFlagController.name}:stateChange`,
},
{
name: 'SelectedNetworkController',
key: `${engine.context.SelectedNetworkController.name}:stateChange`,
Expand Down
51 changes: 0 additions & 51 deletions app/core/redux/slices/featureFlags/index.test.ts

This file was deleted.

77 changes: 0 additions & 77 deletions app/core/redux/slices/featureFlags/index.ts

This file was deleted.

5 changes: 0 additions & 5 deletions app/reducers/index.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import bookmarksReducer from './bookmarks';
import browserReducer from './browser';
import engineReducer from '../core/redux/slices/engine';
import featureFlagsReducer, {
FeatureFlagsState,
} from '../core/redux/slices/featureFlags';
import privacyReducer from './privacy';
import modalsReducer from './modals';
import settingsReducer from './settings';
Expand Down Expand Up @@ -57,7 +54,6 @@ export interface RootState {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
collectibles: any;
engine: { backgroundState: EngineState };
featureFlags: FeatureFlagsState;
joaoloureirop marked this conversation as resolved.
Show resolved Hide resolved
// TODO: Replace "any" with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
privacy: any;
Expand Down Expand Up @@ -135,7 +131,6 @@ const rootReducer = combineReducers<RootState, any>({
// TODO: Replace "any" with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
engine: engineReducer as any,
featureFlags: featureFlagsReducer,
privacy: privacyReducer,
bookmarks: bookmarksReducer,
browser: browserReducer,
Expand Down
21 changes: 21 additions & 0 deletions app/selectors/featureFlagController/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { RootState } from '../../reducers';
import mockedEngine from '../../core/__mocks__/MockedEngine';
import { selectRemoteFeatureFlagControllerState } from '.';
import { mockedState } from './mocks'

jest.mock('../../core/Engine', () => ({
init: () => mockedEngine.init(),
}));

describe('featureFlagController', () => {
afterEach(() => {
jest.clearAllMocks();
});


it('should return feature flag initial state', () => {
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
it('should return feature flag initial state', () => {
it('returns feature flag initial state', () => {

const result = selectRemoteFeatureFlagControllerState(mockedState as RootState);
expect(result).toBeDefined();
});
});

Loading