Skip to content

feat: Show rending confirmation alert on permission page when switching network #30725

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

Merged
merged 8 commits into from
Mar 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ export function validateAddEthereumChainParams(params) {
* @param {object} hooks - The hooks object.
* @param {string} hooks.origin - The origin sending this request.
* @param {boolean} hooks.isAddFlow - Variable to check if its add flow.
* @param {boolean} hooks.isSwitchFlow - Variable to check if its switch flow.
* @param {boolean} [hooks.autoApprove] - A boolean indicating whether the request should prompt the user or be automatically approved.
* @param {Function} hooks.setActiveNetwork - The callback to change the current network for the origin.
* @param {Function} hooks.getCaveat - The callback to get the CAIP-25 caveat for the origin.
Expand All @@ -188,6 +189,7 @@ export async function switchChain(
{
origin,
isAddFlow,
isSwitchFlow,
autoApprove,
setActiveNetwork,
getCaveat,
Expand All @@ -210,9 +212,16 @@ export async function switchChain(
const ethChainIds = getPermittedEthChainIds(caip25Caveat.value);

if (!ethChainIds.includes(chainId)) {
let metadata;
if (isSwitchFlow) {
metadata = {
isSwitchEthereumChain: true,
};
}
await requestPermittedChainsPermissionIncrementalForOrigin({
chainId,
autoApprove,
metadata,
});
} else if (hasApprovalRequestsForOrigin?.() && !isAddFlow) {
await requestUserApproval({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import { Hex } from '@metamask/utils';
import * as EthChainUtils from './ethereum-chain-utils';

describe('Ethereum Chain Utils', () => {
const createMockedSwitchChain = () => {
const createMockedSwitchChain = (mks = {}) => {
const end = jest.fn();
const mocks = {
origin: 'www.test.com',
isAddFlow: false,
isSwitchFlow: false,
autoApprove: false,
setActiveNetwork: jest.fn(),
getCaveat: jest.fn(),
Expand All @@ -23,6 +24,7 @@ describe('Ethereum Chain Utils', () => {
hasApprovalRequestsForOrigin: jest.fn(),
toNetworkConfiguration: {},
fromNetworkConfiguration: {},
...mks,
};
const response: { result?: true } = {};
const switchChain = (chainId: Hex, networkClientId: string) =>
Expand Down Expand Up @@ -177,7 +179,9 @@ describe('Ethereum Chain Utils', () => {

describe('with an existing CAIP-25 permission granted from the multichain flow (isMultichainOrigin: true) and the chainId is not already permissioned', () => {
it('requests permittedChains approval', async () => {
const { mocks, switchChain } = createMockedSwitchChain();
const { mocks, switchChain } = createMockedSwitchChain({
isSwitchFlow: true,
});
mocks.requestPermittedChainsPermissionIncrementalForOrigin.mockRejectedValue(
new Error(
"Cannot switch to or add permissions for chainId '0x1' because permissions were granted over the Multichain API.",
Expand All @@ -194,7 +198,13 @@ describe('Ethereum Chain Utils', () => {

expect(
mocks.requestPermittedChainsPermissionIncrementalForOrigin,
).toHaveBeenCalledWith({ chainId: '0x1', autoApprove: false });
).toHaveBeenCalledWith({
chainId: '0x1',
autoApprove: false,
metadata: {
isSwitchEthereumChain: true,
},
});
});

it('does not switch the active network', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ async function switchEthereumChainHandler(

return switchChain(res, end, chainId, networkClientIdToSwitchTo, {
origin,
isSwitchFlow: true,
setActiveNetwork,
getCaveat,
requestPermittedChainsPermissionIncrementalForOrigin,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ describe('switchEthereumChainHandler', () => {
},
getCaveat: mocks.getCaveat,
hasApprovalRequestsForOrigin: mocks.hasApprovalRequestsForOrigin,
isSwitchFlow: true,
origin: 'example.com',
requestPermittedChainsPermissionIncrementalForOrigin:
mocks.requestPermittedChainsPermissionIncrementalForOrigin,
Expand Down
7 changes: 7 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -5461,11 +5461,13 @@ export default class MetamaskController extends EventEmitter {
* @param {string} options.origin - The origin to request approval for.
* @param {Hex} options.chainId - The chainId to add to the existing permittedChains.
* @param {boolean} options.autoApprove - If the chain should be granted without prompting for user approval.
* @param {object} options.metadata - Request data for the approval.
*/
async requestPermittedChainsPermissionIncremental({
origin,
chainId,
autoApprove,
metadata,
}) {
if (isSnapId(origin)) {
throw new Error(
Expand All @@ -5483,6 +5485,10 @@ export default class MetamaskController extends EventEmitter {
);

if (!autoApprove) {
let options;
if (metadata) {
options = { metadata };
}
await this.permissionController.requestPermissionsIncremental(
{ origin },
{
Expand All @@ -5495,6 +5501,7 @@ export default class MetamaskController extends EventEmitter {
],
},
},
options,
);
return;
}
Expand Down
1 change: 1 addition & 0 deletions app/scripts/metamask-controller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1534,6 +1534,7 @@ describe('MetaMaskController', () => {
).toHaveBeenCalledWith(
{ origin: 'test.com' },
expectedCaip25Permission,
undefined,
);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import React from 'react';
import { useI18nContext } from '../../../hooks/useI18nContext';
import { useTemplateAlertContext } from '../../../pages/confirmations/confirmation/alerts/TemplateAlertContext';
import { Icon, IconName, IconSize } from '../../component-library';
import { PageContainerFooter } from '../../ui/page-container';

export const PermissionPageContainerFooter = ({
cancelText,
disabled,
onCancel,
onSubmit,
}: {
cancelText: string;
disabled: boolean;
onCancel: () => void;
onSubmit: () => void;
}) => {
const t = useI18nContext();
const { hasAlerts, showAlertsModal } = useTemplateAlertContext();

return (
<PageContainerFooter
footerClassName="permission-page-container-footer"
cancelButtonType="default"
onCancel={onCancel}
cancelText={cancelText}
onSubmit={hasAlerts ? showAlertsModal : onSubmit}
submitText={t('confirm')}
buttonSizeLarge={false}
disabled={disabled}
submitButtonIcon={
hasAlerts ? <Icon name={IconName.Info} size={IconSize.Sm} /> : undefined
}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
import { Caip25EndowmentPermissionName } from '@metamask/multichain';
import { SubjectType } from '@metamask/permission-controller';
import { MetaMetricsEventCategory } from '../../../../shared/constants/metametrics';
import { PageContainerFooter } from '../../ui/page-container';
import PermissionsConnectFooter from '../permissions-connect-footer';
import { RestrictedMethods } from '../../../../shared/constants/permissions';

Expand All @@ -24,7 +23,9 @@ import {
getRequestedCaip25CaveatValue,
getCaip25PermissionsResponse,
} from '../../../pages/permissions-connect/connect-page/utils';
import { TemplateAlertContextProvider } from '../../../pages/confirmations/confirmation/alerts/TemplateAlertContext';
import { containsEthPermissionsAndNonEvmAccount } from '../../../helpers/utils/permissions';
import { PermissionPageContainerFooter } from './permission-page-container-footer.component';
import { PermissionPageContainerContent } from '.';

export default class PermissionPageContainer extends Component {
Expand Down Expand Up @@ -211,7 +212,10 @@ export default class PermissionPageContainer extends Component {
: this.context.t('back');

return (
<>
<TemplateAlertContextProvider
onSubmit={() => this.onSubmit()}
confirmationId={request?.metadata?.id}
>
{this.state.isShowingSnapsPrivacyWarning && (
<SnapPrivacyWarning
onAccepted={() => confirmSnapsPrivacyWarning()}
Expand All @@ -235,21 +239,17 @@ export default class PermissionPageContainer extends Component {
{targetSubjectMetadata?.subjectType !== SubjectType.Snap && (
<PermissionsConnectFooter />
)}
<PageContainerFooter
footerClassName="permission-page-container-footer"
cancelButtonType="default"
<PermissionPageContainerFooter
onCancel={() => this.onLeftFooterClick()}
cancelText={footerLeftActionText}
onSubmit={() => this.onSubmit()}
submitText={this.context.t('confirm')}
buttonSizeLarge={false}
disabled={containsEthPermissionsAndNonEvmAccount(
selectedAccounts,
requestedPermissions,
)}
/>
</Box>
</>
</TemplateAlertContextProvider>
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export default class PageContainerFooter extends Component {
buttonSizeLarge: PropTypes.bool,
footerClassName: PropTypes.string,
footerButtonClassName: PropTypes.string,
submitButtonIcon: PropTypes.string,
};

static contextTypes = {
Expand All @@ -37,6 +38,7 @@ export default class PageContainerFooter extends Component {
buttonSizeLarge = false,
footerClassName,
footerButtonClassName,
submitButtonIcon,
} = this.props;

return (
Expand Down Expand Up @@ -68,6 +70,7 @@ export default class PageContainerFooter extends Component {
disabled={disabled}
onClick={(e) => onSubmit(e)}
data-testid="page-container-footer-next"
icon={submitButtonIcon}
>
{submitText || this.context.t('next')}
</Button>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React from 'react';
import { fireEvent } from '@testing-library/react';

import { renderWithProvider } from '../../../../../test/lib/render-helpers';
import { Icon, IconName } from '../../../component-library';
import PageFooter from '.';

describe('Page Footer', () => {
Expand Down Expand Up @@ -72,5 +74,18 @@ describe('Page Footer', () => {
console.log(submitButton.className);
expect(submitButton.className).toContain('danger-primary');
});

it('renders submitButtonIcon if passed', () => {
const { getByTestId } = renderWithProvider(
<PageFooter
{...props}
submitButtonIcon={
<Icon name={IconName.Add} data-testid="icon-test-id" />
}
/>,
);

expect(getByTestId('icon-test-id')).toBeInTheDocument();
});
});
});
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import { ApprovalRequest } from '@metamask/approval-controller';
import React, {
ReactElement,
createContext,
useCallback,
useContext,
useState,
} from 'react';
import { useSelector } from 'react-redux';

import useAlerts from '../../../../hooks/useAlerts';
import { AlertActionHandlerProvider } from '../../../../components/app/alert-system/contexts/alertActionHandler';
import { AlertMetricsProvider } from '../../../../components/app/alert-system/contexts/alertMetricsContext';
import { MultipleAlertModal } from '../../../../components/app/alert-system/multiple-alert-modal';
import { getMemoizedUnapprovedConfirmations } from '../../../../selectors';
import { useTemplateConfirmationAlerts } from './useTemplateConfirmationAlerts';
import { useAlertsActions } from './useAlertsActions';

Expand All @@ -27,9 +28,16 @@ export const TemplateAlertContext = createContext<

export const TemplateAlertContextProvider: React.FC<{
children: ReactElement;
pendingConfirmation: ApprovalRequest<{ id: string }>;
confirmationId: string;
onSubmit: () => void;
}> = ({ children, pendingConfirmation, onSubmit }) => {
}> = ({ children, confirmationId, onSubmit }) => {
const pendingConfirmations = useSelector(getMemoizedUnapprovedConfirmations);

const pendingConfirmation =
pendingConfirmations?.find(
(confirmation) => confirmation.id === confirmationId,
) ?? pendingConfirmations[0];

const [isAlertsModalVisible, setAlertsModalVisible] = useState(false);
const alertOwnerId = pendingConfirmation?.id;
useTemplateConfirmationAlerts(pendingConfirmation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,18 @@ const PENDING_APPROVAL_MOCK = {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as ApprovalRequest<any>;

const PERMISSION_MOCK = {
id: 'testApprovalId',
requestData: {
testProperty: 'testValue',
metadata: { isSwitchEthereumChain: true },
},
type: ApprovalType.WalletRequestPermissions,
origin: 'https://metamask.github.io',
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as unknown as ApprovalRequest<any>;

const ADD_ETH_CHAIN_ALERT = [
{
actions: [
Expand Down Expand Up @@ -83,6 +95,27 @@ describe('useUpdateEthereumChainAlerts', () => {
expect(result.current).toStrictEqual(SWITCH_ETH_CHAIN_ALERT);
});

it('returns alert for permission request with isLegacySwitchEthereumChain if there are pending confirmations', () => {
const { result } = renderHookWithProvider(
() =>
useUpdateEthereumChainAlerts({
...PERMISSION_MOCK,
type: ApprovalType.SwitchEthereumChain,
}),
{
...state,
metamask: {
...state.metamask,
pendingApprovals: {
...state.metamask.pendingApprovals,
[PERMISSION_MOCK.id]: PERMISSION_MOCK,
},
},
},
);
expect(result.current).toStrictEqual(SWITCH_ETH_CHAIN_ALERT);
});

it('does not returns alert if there are no pending confirmations', () => {
const { result } = renderHookWithProvider(
() => useUpdateEthereumChainAlerts(PENDING_APPROVAL_MOCK),
Expand Down
Loading
Loading