From ac1bb1db869dc69bcd5acf13c20dd596bc7a0d52 Mon Sep 17 00:00:00 2001 From: jpuri Date: Tue, 4 Mar 2025 15:35:05 +0530 Subject: [PATCH 1/2] Show rending confirmation alert on permission page when switching network. --- .../handlers/ethereum-chain-utils.js | 9 +++++ .../handlers/ethereum-chain-utils.test.ts | 16 +++++++-- .../handlers/switch-ethereum-chain.js | 1 + .../handlers/switch-ethereum-chain.test.js | 1 + app/scripts/metamask-controller.js | 7 ++++ app/scripts/metamask-controller.test.js | 1 + ...ission-page-container-footer.component.tsx | 36 +++++++++++++++++++ .../permission-page-container.component.js | 16 ++++----- .../page-container-footer.component.js | 3 ++ .../page-container-footer.component.test.js | 15 ++++++++ .../alerts/TemplateAlertContext.tsx | 14 ++++++-- .../useUpdateEthereumChainAlerts.test.ts | 33 +++++++++++++++++ .../alerts/useUpdateEthereumChainAlerts.ts | 21 +++++++---- .../confirmation/confirmation.js | 2 +- 14 files changed, 154 insertions(+), 21 deletions(-) create mode 100644 ui/components/app/permission-page-container/permission-page-container-footer.component.tsx diff --git a/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js b/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js index 94ad35335236..3c69d83e56f5 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js @@ -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. @@ -188,6 +189,7 @@ export async function switchChain( { origin, isAddFlow, + isSwitchFlow, autoApprove, setActiveNetwork, getCaveat, @@ -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({ diff --git a/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.test.ts b/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.test.ts index 9734e043a80e..b1ec4b8dbacf 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.test.ts +++ b/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.test.ts @@ -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(), @@ -23,6 +24,7 @@ describe('Ethereum Chain Utils', () => { hasApprovalRequestsForOrigin: jest.fn(), toNetworkConfiguration: {}, fromNetworkConfiguration: {}, + ...mks, }; const response: { result?: true } = {}; const switchChain = (chainId: Hex, networkClientId: string) => @@ -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.", @@ -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 () => { diff --git a/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.js b/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.js index 938d20f174b7..3bf690e71837 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.js @@ -77,6 +77,7 @@ async function switchEthereumChainHandler( return switchChain(res, end, chainId, networkClientIdToSwitchTo, { origin, + isSwitchFlow: true, setActiveNetwork, getCaveat, requestPermittedChainsPermissionIncrementalForOrigin, diff --git a/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.test.js b/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.test.js index 43c8ba54e321..3c3792605c53 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.test.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/switch-ethereum-chain.test.js @@ -180,6 +180,7 @@ describe('switchEthereumChainHandler', () => { }, getCaveat: mocks.getCaveat, hasApprovalRequestsForOrigin: mocks.hasApprovalRequestsForOrigin, + isSwitchFlow: true, origin: 'example.com', requestPermittedChainsPermissionIncrementalForOrigin: mocks.requestPermittedChainsPermissionIncrementalForOrigin, diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 86bc7997b4b1..6a6a71a33a98 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -5425,11 +5425,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( @@ -5447,6 +5449,10 @@ export default class MetamaskController extends EventEmitter { ); if (!autoApprove) { + let options; + if (metadata) { + options = { metadata }; + } await this.permissionController.requestPermissionsIncremental( { origin }, { @@ -5459,6 +5465,7 @@ export default class MetamaskController extends EventEmitter { ], }, }, + options, ); return; } diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index 5121682e9b85..b263e76ff148 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -1534,6 +1534,7 @@ describe('MetaMaskController', () => { ).toHaveBeenCalledWith( { origin: 'test.com' }, expectedCaip25Permission, + undefined, ); }); diff --git a/ui/components/app/permission-page-container/permission-page-container-footer.component.tsx b/ui/components/app/permission-page-container/permission-page-container-footer.component.tsx new file mode 100644 index 000000000000..f8bd63ba9c0b --- /dev/null +++ b/ui/components/app/permission-page-container/permission-page-container-footer.component.tsx @@ -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 ( + : undefined + } + /> + ); +}; diff --git a/ui/components/app/permission-page-container/permission-page-container.component.js b/ui/components/app/permission-page-container/permission-page-container.component.js index 96d80c056563..4bbb8523b509 100644 --- a/ui/components/app/permission-page-container/permission-page-container.component.js +++ b/ui/components/app/permission-page-container/permission-page-container.component.js @@ -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'; @@ -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 { @@ -211,7 +212,10 @@ export default class PermissionPageContainer extends Component { : this.context.t('back'); return ( - <> + this.onSubmit()} + confirmationId={request?.metadata?.id} + > {this.state.isShowingSnapsPrivacyWarning && ( confirmSnapsPrivacyWarning()} @@ -235,21 +239,17 @@ export default class PermissionPageContainer extends Component { {targetSubjectMetadata?.subjectType !== SubjectType.Snap && ( )} - this.onLeftFooterClick()} cancelText={footerLeftActionText} onSubmit={() => this.onSubmit()} - submitText={this.context.t('confirm')} - buttonSizeLarge={false} disabled={containsEthPermissionsAndNonEvmAccount( selectedAccounts, requestedPermissions, )} /> - + ); } } diff --git a/ui/components/ui/page-container/page-container-footer/page-container-footer.component.js b/ui/components/ui/page-container/page-container-footer/page-container-footer.component.js index d92cc5916c74..a9bcce0a10bb 100644 --- a/ui/components/ui/page-container/page-container-footer/page-container-footer.component.js +++ b/ui/components/ui/page-container/page-container-footer/page-container-footer.component.js @@ -17,6 +17,7 @@ export default class PageContainerFooter extends Component { buttonSizeLarge: PropTypes.bool, footerClassName: PropTypes.string, footerButtonClassName: PropTypes.string, + submitButtonIcon: PropTypes.string, }; static contextTypes = { @@ -37,6 +38,7 @@ export default class PageContainerFooter extends Component { buttonSizeLarge = false, footerClassName, footerButtonClassName, + submitButtonIcon, } = this.props; return ( @@ -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')} diff --git a/ui/components/ui/page-container/page-container-footer/page-container-footer.component.test.js b/ui/components/ui/page-container/page-container-footer/page-container-footer.component.test.js index 69eaa48860fb..2ebfbf525b7f 100644 --- a/ui/components/ui/page-container/page-container-footer/page-container-footer.component.test.js +++ b/ui/components/ui/page-container/page-container-footer/page-container-footer.component.test.js @@ -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', () => { @@ -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( + + } + />, + ); + + expect(getByTestId('icon-test-id')).toBeInTheDocument(); + }); }); }); diff --git a/ui/pages/confirmations/confirmation/alerts/TemplateAlertContext.tsx b/ui/pages/confirmations/confirmation/alerts/TemplateAlertContext.tsx index 268b03e2dc40..0ba7be09a4a3 100644 --- a/ui/pages/confirmations/confirmation/alerts/TemplateAlertContext.tsx +++ b/ui/pages/confirmations/confirmation/alerts/TemplateAlertContext.tsx @@ -1,4 +1,3 @@ -import { ApprovalRequest } from '@metamask/approval-controller'; import React, { ReactElement, createContext, @@ -6,11 +5,13 @@ import React, { 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'; @@ -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); diff --git a/ui/pages/confirmations/confirmation/alerts/useUpdateEthereumChainAlerts.test.ts b/ui/pages/confirmations/confirmation/alerts/useUpdateEthereumChainAlerts.test.ts index d11ebed300d1..3a6331c2074a 100644 --- a/ui/pages/confirmations/confirmation/alerts/useUpdateEthereumChainAlerts.test.ts +++ b/ui/pages/confirmations/confirmation/alerts/useUpdateEthereumChainAlerts.test.ts @@ -22,6 +22,18 @@ const PENDING_APPROVAL_MOCK = { // eslint-disable-next-line @typescript-eslint/no-explicit-any } as ApprovalRequest; +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; + const ADD_ETH_CHAIN_ALERT = [ { actions: [ @@ -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), diff --git a/ui/pages/confirmations/confirmation/alerts/useUpdateEthereumChainAlerts.ts b/ui/pages/confirmations/confirmation/alerts/useUpdateEthereumChainAlerts.ts index a9c70a55d57a..bec8d4cc46ee 100644 --- a/ui/pages/confirmations/confirmation/alerts/useUpdateEthereumChainAlerts.ts +++ b/ui/pages/confirmations/confirmation/alerts/useUpdateEthereumChainAlerts.ts @@ -1,5 +1,6 @@ import { ApprovalRequest } from '@metamask/approval-controller'; import { ApprovalType } from '@metamask/controller-utils'; +import { Json } from '@metamask/utils'; import { useMemo } from 'react'; import { useSelector } from 'react-redux'; @@ -18,7 +19,7 @@ const VALIDATED_APPROVAL_TYPES = [ ]; export function useUpdateEthereumChainAlerts( - pendingConfirmation: ApprovalRequest<{ id: string }>, + pendingConfirmation: ApprovalRequest>, ): Alert[] { const pendingConfirmationsFromOrigin = useSelector((state) => getApprovalsByOrigin( @@ -26,14 +27,17 @@ export function useUpdateEthereumChainAlerts( pendingConfirmation?.origin, ), ); - const t = useI18nContext(); + + console.log(' pendingConfirmation = ', pendingConfirmation); return useMemo(() => { if ( pendingConfirmationsFromOrigin?.length <= 1 || - !VALIDATED_APPROVAL_TYPES.includes( - pendingConfirmation?.type as ApprovalType, - ) + (!VALIDATED_APPROVAL_TYPES.includes( + pendingConfirmation.type as ApprovalType, + ) && + (pendingConfirmation?.requestData?.metadata as Record) + ?.isSwitchEthereumChain !== true) ) { return []; } @@ -57,5 +61,10 @@ export function useUpdateEthereumChainAlerts( severity: Severity.Warning, }, ]; - }, [pendingConfirmation?.type, pendingConfirmationsFromOrigin?.length, t]); + }, [ + pendingConfirmation?.type, + pendingConfirmation?.requestData?.metadata, + pendingConfirmationsFromOrigin?.length, + t, + ]); } diff --git a/ui/pages/confirmations/confirmation/confirmation.js b/ui/pages/confirmations/confirmation/confirmation.js index 3a21d88694af..393c81b9a567 100644 --- a/ui/pages/confirmations/confirmation/confirmation.js +++ b/ui/pages/confirmations/confirmation/confirmation.js @@ -496,7 +496,7 @@ export default function ConfirmationPage({ return (
From 6e0cf453e5b162f2dbff77bb9f09f6d911acb5bc Mon Sep 17 00:00:00 2001 From: jpuri Date: Thu, 13 Mar 2025 17:10:57 +0530 Subject: [PATCH 2/2] update --- .../confirmation/alerts/useUpdateEthereumChainAlerts.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/ui/pages/confirmations/confirmation/alerts/useUpdateEthereumChainAlerts.ts b/ui/pages/confirmations/confirmation/alerts/useUpdateEthereumChainAlerts.ts index bec8d4cc46ee..6c852f372fc4 100644 --- a/ui/pages/confirmations/confirmation/alerts/useUpdateEthereumChainAlerts.ts +++ b/ui/pages/confirmations/confirmation/alerts/useUpdateEthereumChainAlerts.ts @@ -29,7 +29,6 @@ export function useUpdateEthereumChainAlerts( ); const t = useI18nContext(); - console.log(' pendingConfirmation = ', pendingConfirmation); return useMemo(() => { if ( pendingConfirmationsFromOrigin?.length <= 1 ||