Skip to content

Commit

Permalink
feat: Remove 'confirmation redesign' developer settings toggle (#29873)
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**

- Delete Confirmations Redesign developer options settings toggle.
- Delete corresponding selector and action.
- Delete `isRedesignedConfirmationsDeveloperEnabled` preferences state
property with a migration.
- Remove usage of `ENABLE_CONFIRMATION_REDESIGN` environment variable.
- Adjusted tests

<!--
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/29873?quickstart=1)

## **Related issues**

Fixes: MetaMask/MetaMask-planning#3708

## **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
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/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-extension/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
pedronfigueiredo authored Jan 24, 2025
1 parent 2263ce4 commit b810b5b
Show file tree
Hide file tree
Showing 48 changed files with 141 additions and 608 deletions.
3 changes: 0 additions & 3 deletions .metamaskrc.dist
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ BLOCKAID_PUBLIC_KEY=
; Set this to true to enable the snap path for the Firefox WebDriver (Linux)
; FIREFOX_SNAP=

; Enable the redesigned confirmations still in development, without needing to toggle the developer setting.
; ENABLE_CONFIRMATION_REDESIGN=

; URL of security alerts API used to validate dApp requests
; SECURITY_ALERTS_API_URL='http://localhost:3000'
; Temporary mechanism to enable security alerts API prior to release
Expand Down
1 change: 0 additions & 1 deletion .storybook/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ module.exports = {
staticDirs: ['../app', './images'],
env: (config) => ({
...config,
ENABLE_CONFIRMATION_REDESIGN: true,
INFURA_PROJECT_ID: process.env.INFURA_STORYBOOK_PROJECT_ID || '',
}),
// Uses babel.config.js settings and prevents "Missing class properties transform" error
Expand Down
1 change: 0 additions & 1 deletion app/scripts/constants/sentry-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ export const SENTRY_BACKGROUND_STATE = {
preferences: {
autoLockTimeLimit: true,
hideZeroBalanceTokens: true,
isRedesignedConfirmationsDeveloperEnabled: false,
showExtensionInFullSizeView: true,
showFiatInTestnets: true,
showTestNetworks: true,
Expand Down
11 changes: 0 additions & 11 deletions app/scripts/controllers/preferences-controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,15 +334,6 @@ describe('preferences controller', () => {
});
});

describe('isRedesignedConfirmationsFeatureEnabled', () => {
const { controller } = setupController({});
it('isRedesignedConfirmationsFeatureEnabled should default to false', () => {
expect(
controller.state.preferences.isRedesignedConfirmationsDeveloperEnabled,
).toStrictEqual(false);
});
});

describe('setUseSafeChainsListValidation', function () {
const { controller } = setupController({});
it('should default to true', function () {
Expand Down Expand Up @@ -727,7 +718,6 @@ describe('preferences controller', () => {
petnamesEnabled: true,
shouldShowAggregatedBalancePopover: true,
featureNotificationsEnabled: false,
isRedesignedConfirmationsDeveloperEnabled: false,
showConfirmationAdvancedDetails: false,
showMultiRpcModal: false,
showNativeTokenAsMainBalance: false,
Expand Down Expand Up @@ -756,7 +746,6 @@ describe('preferences controller', () => {
privacyMode: false,
shouldShowAggregatedBalancePopover: true,
featureNotificationsEnabled: false,
isRedesignedConfirmationsDeveloperEnabled: false,
showConfirmationAdvancedDetails: true,
showMultiRpcModal: false,
showNativeTokenAsMainBalance: false,
Expand Down
2 changes: 0 additions & 2 deletions app/scripts/controllers/preferences-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ export type Preferences = {
featureNotificationsEnabled: boolean;
showMultiRpcModal: boolean;
privacyMode: boolean;
isRedesignedConfirmationsDeveloperEnabled: boolean;
showConfirmationAdvancedDetails: boolean;
tokenSortConfig: {
key: string;
Expand Down Expand Up @@ -221,7 +220,6 @@ export const getDefaultPreferencesControllerState =
hideZeroBalanceTokens: false,
petnamesEnabled: true,
featureNotificationsEnabled: false,
isRedesignedConfirmationsDeveloperEnabled: false,
showConfirmationAdvancedDetails: false,
showMultiRpcModal: false,
privacyMode: false,
Expand Down
1 change: 0 additions & 1 deletion app/scripts/fixtures/with-preferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export const FIXTURES_PREFERENCES = {
featureNotificationsEnabled: true,
showTokenAutodetectModal: false,
showNftAutodetectModal: false,
isRedesignedConfirmationsDeveloperEnabled: false,
showConfirmationAdvancedDetails: false,
privacyMode: false,
},
Expand Down
4 changes: 0 additions & 4 deletions app/scripts/lib/createRPCMethodTrackingMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ function finalizeSignatureFragment(
* that should be tracked for methods rate limited by random sample.
* @param {Function} opts.getAccountType
* @param {Function} opts.getDeviceModel
* @param {Function} opts.isRedesignedConfirmationsDeveloperEnabled
* @param {RestrictedControllerMessenger} opts.snapAndHardwareMessenger
* @param {number} [opts.globalRateLimitTimeout] - time, in milliseconds, of the sliding
* time window that should limit the number of method calls tracked to globalRateLimitMaxAmount.
Expand All @@ -213,7 +212,6 @@ export default function createRPCMethodTrackingMiddleware({
globalRateLimitMaxAmount = 10, // max of events in the globalRateLimitTimeout window. pass 0 for no global rate limit
getAccountType,
getDeviceModel,
isRedesignedConfirmationsDeveloperEnabled,
snapAndHardwareMessenger,
appStateController,
metaMetricsController,
Expand Down Expand Up @@ -318,8 +316,6 @@ export default function createRPCMethodTrackingMiddleware({
if (
shouldUseRedesignForSignatures({
approvalType: MESSAGE_TYPE_TO_APPROVAL_TYPE[method],
isRedesignedConfirmationsDeveloperEnabled:
isRedesignedConfirmationsDeveloperEnabled(),
})
) {
eventProperties.ui_customizations = [
Expand Down
13 changes: 0 additions & 13 deletions app/scripts/lib/createRPCMethodTrackingMiddleware.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ const createHandler = (opts) =>
globalRateLimitMaxAmount: 0,
appStateController,
metaMetricsController,
isRedesignedConfirmationsDeveloperEnabled: () => false,
...opts,
});

Expand Down Expand Up @@ -213,22 +212,10 @@ describe('createRPCMethodTrackingMiddleware', () => {
});

describe('participateInMetaMetrics is set to true', () => {
const originalEnableConfirmationRedesign =
process.env.ENABLE_CONFIRMATION_REDESIGN;

beforeEach(() => {
metaMetricsController.setParticipateInMetaMetrics(true);
});

beforeAll(() => {
process.env.ENABLE_CONFIRMATION_REDESIGN = 'false';
});

afterAll(() => {
process.env.ENABLE_CONFIRMATION_REDESIGN =
originalEnableConfirmationRedesign;
});

it(`should immediately track a ${MetaMetricsEventName.SignatureRequested} event`, async () => {
const req = {
id: MOCK_ID,
Expand Down
1 change: 0 additions & 1 deletion app/scripts/lib/transaction/metrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ const mockTransactionMetricsRequest = {
getIsSmartTransaction: jest.fn(),
getSmartTransactionByMinedTxHash: jest.fn(),
getMethodData: jest.fn(),
getIsRedesignedConfirmationsDeveloperEnabled: jest.fn(),
getIsConfirmationAdvancedDetailsOpen: jest.fn(),
} as TransactionMetricsRequest;

Expand Down
3 changes: 0 additions & 3 deletions app/scripts/lib/transaction/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ export type TransactionMetricsRequest = {
txhash: string | undefined,
) => SmartTransaction;
getMethodData: (data: string) => Promise<{ name: string }>;
getIsRedesignedConfirmationsDeveloperEnabled: () => boolean;
getIsConfirmationAdvancedDetailsOpen: () => boolean;
};

Expand Down Expand Up @@ -1023,8 +1022,6 @@ async function buildEventFragmentProperties({

const isRedesignedForTransaction = shouldUseRedesignForTransactions({
transactionMetadataType: transactionMeta.type as TransactionType,
isRedesignedConfirmationsDeveloperEnabled:
transactionMetricsRequest.getIsRedesignedConfirmationsDeveloperEnabled(),
});
if (isRedesignedForTransaction) {
uiCustomizations.push(
Expand Down
9 changes: 0 additions & 9 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -6309,8 +6309,6 @@ export default class MetamaskController extends EventEmitter {
createRPCMethodTrackingMiddleware({
getAccountType: this.getAccountType.bind(this),
getDeviceModel: this.getDeviceModel.bind(this),
isRedesignedConfirmationsDeveloperEnabled:
this.isConfirmationRedesignDeveloperEnabled.bind(this),
snapAndHardwareMessenger: this.controllerMessenger.getRestricted({
name: 'SnapAndHardwareMessenger',
allowedActions: [
Expand Down Expand Up @@ -6965,11 +6963,6 @@ export default class MetamaskController extends EventEmitter {
});
}

isConfirmationRedesignDeveloperEnabled() {
return this.preferencesController.state.preferences
.isRedesignedConfirmationsDeveloperEnabled;
}

/**
* The chain list is fetched live at runtime, falling back to a cache.
* This preseeds the cache at startup with a static list provided at build.
Expand Down Expand Up @@ -7171,8 +7164,6 @@ export default class MetamaskController extends EventEmitter {
this.provider,
);
},
getIsRedesignedConfirmationsDeveloperEnabled:
this.isConfirmationRedesignDeveloperEnabled.bind(this),
getIsConfirmationAdvancedDetailsOpen: () => {
return this.preferencesController.state.preferences
.showConfirmationAdvancedDetails;
Expand Down
79 changes: 79 additions & 0 deletions app/scripts/migrations/142.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import { migrate, version } from './142';

const oldVersion = 141;

describe(`migration #${version}`, () => {
it('updates the version metadata', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {},
};
const newStorage = await migrate(oldStorage);
expect(newStorage.meta).toStrictEqual({ version });
});

describe(`migration #${version}`, () => {
it('removes the isRedesignedConfirmationsDeveloperEnabled preference if it is set to true', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
PreferencesController: {
preferences: {
isRedesignedConfirmationsDeveloperEnabled: true,
},
},
},
};
const expectedData = {
PreferencesController: {
preferences: {},
},
};
const newStorage = await migrate(oldStorage);

expect(newStorage.data).toStrictEqual(expectedData);
});

it('removes the isRedesignedConfirmationsDeveloperEnabled preference if it is set to false', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
PreferencesController: {
preferences: {
isRedesignedConfirmationsDeveloperEnabled: false,
},
},
},
};
const expectedData = {
PreferencesController: {
preferences: {},
},
};
const newStorage = await migrate(oldStorage);

expect(newStorage.data).toStrictEqual(expectedData);
});

it('does nothing to other PreferencesController state if there is not a isRedesignedConfirmationsDeveloperEnabled preference', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
PreferencesController: {
existingPreference: true,
},
},
};

const expectedData = {
PreferencesController: {
existingPreference: true,
},
};

const newStorage = await migrate(oldStorage);

expect(newStorage.data).toStrictEqual(expectedData);
});
});
});
41 changes: 41 additions & 0 deletions app/scripts/migrations/142.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { cloneDeep } from 'lodash';

type VersionedData = {
meta: { version: number };
data: Record<string, unknown>;
};

export const version = 142;

/**
* This migration deletes the preference `isRedesignedConfirmationsDeveloperEnabled` if the
* user has existing data.
*
* @param originalVersionedData - Versioned MetaMask extension state, exactly
* what we persist to dist.
* @param originalVersionedData.meta - State metadata.
* @param originalVersionedData.meta.version - The current state version.
* @param originalVersionedData.data - The persisted MetaMask state, keyed by
* controller.
* @returns Updated versioned MetaMask extension state.
*/
export async function migrate(
originalVersionedData: VersionedData,
): Promise<VersionedData> {
const versionedData = cloneDeep(originalVersionedData);
versionedData.meta.version = version;
transformState(versionedData.data);
return versionedData;
}

function transformState(state: Record<string, unknown>) {
const preferencesControllerState = state?.PreferencesController as
| Record<string, unknown>
| undefined;

const preferences = preferencesControllerState?.preferences as
| Record<string, unknown>
| undefined;

delete preferences?.isRedesignedConfirmationsDeveloperEnabled;
}
1 change: 1 addition & 0 deletions app/scripts/migrations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ const migrations = [
require('./139'),
require('./140'),
require('./141'),
require('./142'),
];

export default migrations;
2 changes: 0 additions & 2 deletions builds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,6 @@ env:
# Variables that are modified with hardcoded code
###

# Used to enable confirmation redesigned pages
- ENABLE_CONFIRMATION_REDESIGN: ''
# URL of the decoding API used to provide additional data from signature requests
- DECODING_API_URL: 'https://signature-insights.api.cx.metamask.io/v1'
# Determines if feature flagged Settings Page - Developer Options should be used
Expand Down
Loading

0 comments on commit b810b5b

Please sign in to comment.