From 231f780b787bfc75e51678028ec3200c46357209 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Mon, 14 Jul 2025 15:21:56 +0200 Subject: [PATCH 01/31] (wip) refactor: use `@metamask/eth-qr-keyring` --- app/actions/modals/index.js | 7 + .../TransactionApproval.tsx | 19 +- .../UI/QRHardware/AnimatedQRScanner.tsx | 9 +- .../UI/QRHardware/QRSigningDetails.tsx | 27 +- .../UI/QRHardware/QRSigningModal/index.tsx | 8 +- app/components/UI/QRHardware/types.tsx | 14 - .../UI/QRHardware/withQRHardwareAwareness.tsx | 50 +--- .../Views/AccountActions/AccountActions.tsx | 7 +- .../Views/ConnectQRHardware/index.tsx | 268 ++++++------------ .../components/qr-info/qr-info.tsx | 94 +++--- .../qr-hardware-context.test.tsx | 44 ++- .../qr-hardware-context.tsx | 28 +- .../useQRHardwareAwareness.test.ts | 27 +- .../useQRHardwareAwareness.ts | 46 +-- app/core/Engine/Engine.ts | 66 ++++- app/core/QrKeyring/QrKeyring.ts | 34 +++ .../redux/slices/qrKeyringScanner/index.ts | 33 +++ app/reducers/index.ts | 3 + app/util/test/initial-root-state.ts | 2 + package.json | 8 +- yarn.lock | 83 ++---- 21 files changed, 396 insertions(+), 481 deletions(-) delete mode 100644 app/components/UI/QRHardware/types.tsx create mode 100644 app/core/QrKeyring/QrKeyring.ts create mode 100644 app/core/redux/slices/qrKeyringScanner/index.ts diff --git a/app/actions/modals/index.js b/app/actions/modals/index.js index 8d382f2b70e3..d9d997be5a6f 100644 --- a/app/actions/modals/index.js +++ b/app/actions/modals/index.js @@ -31,3 +31,10 @@ export function toggleSignModal(show) { show, }; } + +export function toggleQrKeyringScanModal(show) { + return { + type: 'TOGGLE_QR_KEYRING_SCAN_MODAL', + show, + }; +} diff --git a/app/components/Approvals/TransactionApproval/TransactionApproval.tsx b/app/components/Approvals/TransactionApproval/TransactionApproval.tsx index 8a52bb36c7cd..a6ba56e41b2c 100644 --- a/app/components/Approvals/TransactionApproval/TransactionApproval.tsx +++ b/app/components/Approvals/TransactionApproval/TransactionApproval.tsx @@ -5,8 +5,8 @@ import Approval from '../../Views/confirmations/legacy/Approval'; import Approve from '../../Views/confirmations/legacy/ApproveView/Approve'; import QRSigningModal from '../../UI/QRHardware/QRSigningModal'; import withQRHardwareAwareness from '../../UI/QRHardware/withQRHardwareAwareness'; -import { IQRState } from '../../UI/QRHardware/types'; import { useConfirmationRedesignEnabled } from '../../Views/confirmations/hooks/useConfirmationRedesignEnabled'; +import { QrScanRequest, QrScanRequestType } from '@metamask/eth-qr-keyring'; export enum TransactionModalType { Transaction = 'transaction', @@ -19,15 +19,22 @@ export interface TransactionApprovalProps { // eslint-disable-next-line @typescript-eslint/no-explicit-any navigation: any; onComplete: () => void; - QRState?: IQRState; + pendingScanRequest?: QrScanRequest; isSigningQRObject?: boolean; } const TransactionApprovalInternal = (props: TransactionApprovalProps) => { const { approvalRequest } = useApprovalRequest(); const { isRedesignedEnabled } = useConfirmationRedesignEnabled(); - const { onComplete: propsOnComplete } = props; - const isQRSigning = props.isSigningQRObject && !props.transactionType; + const { + onComplete: propsOnComplete, + pendingScanRequest, + isSigningQRObject, + } = props; + const isQRSigning = + isSigningQRObject && + pendingScanRequest?.type === QrScanRequestType.SIGN && + !props.transactionType; const onComplete = useCallback(() => { propsOnComplete(); @@ -64,9 +71,7 @@ const TransactionApprovalInternal = (props: TransactionApprovalProps) => { return ( StyleSheet.create({ @@ -95,7 +96,7 @@ const frameImage = require('../../../images/frame.png'); // eslint-disable-line interface AnimatedQRScannerProps { visible: boolean; - purpose: 'sync' | 'sign'; + purpose: QrScanRequestType; onScanSuccess: (ur: UR) => void; onScanError: (error: string) => void; hideModal: () => void; @@ -122,7 +123,7 @@ const AnimatedQRScannerModal = (props: AnimatedQRScannerProps) => { const { hasPermission, requestPermission } = useCameraPermission(); let expectedURTypes: string[]; - if (purpose === 'sync') { + if (purpose === QrScanRequestType.PAIR) { expectedURTypes = [ SUPPORTED_UR_TYPE.CRYPTO_HDKEY, SUPPORTED_UR_TYPE.CRYPTO_ACCOUNT, @@ -148,7 +149,7 @@ const AnimatedQRScannerModal = (props: AnimatedQRScannerProps) => { {strings('connect_qr_hardware.hint_text')} {strings( - purpose === 'sync' + purpose === QrScanRequestType.PAIR ? 'connect_qr_hardware.purpose_connect' : 'connect_qr_hardware.purpose_sign', )} @@ -198,7 +199,7 @@ const AnimatedQRScannerModal = (props: AnimatedQRScannerProps) => { onScanSuccess(ur); setProgress(0); setURDecoder(new URRegistryDecoder()); - } else if (purpose === 'sync') { + } else if (purpose === QrScanRequestType.PAIR) { trackEvent( createEventBuilder(MetaMetricsEvents.HARDWARE_WALLET_ERROR) .addProperties({ diff --git a/app/components/UI/QRHardware/QRSigningDetails.tsx b/app/components/UI/QRHardware/QRSigningDetails.tsx index 634109398b67..783b9591b6e2 100644 --- a/app/components/UI/QRHardware/QRSigningDetails.tsx +++ b/app/components/UI/QRHardware/QRSigningDetails.tsx @@ -23,7 +23,6 @@ import AnimatedQRScannerModal from './AnimatedQRScanner'; import { fontStyles } from '../../../styles/common'; import AccountInfoCard from '../AccountInfoCard'; import ActionView from '../ActionView'; -import { IQRState } from './types'; import { UR } from '@ngraveio/bc-ur'; import { ETHSignature } from '@keystonehq/bc-ur-registry-eth'; import { stringify as uuidStringify } from 'uuid'; @@ -34,9 +33,10 @@ import { useNavigation } from '@react-navigation/native'; import { useTheme } from '../../../util/theme'; import Device from '../../../util/device'; import { useMetrics } from '../../../components/hooks/useMetrics'; +import { QrScanRequest, QrScanRequestType } from '@metamask/eth-qr-keyring'; interface IQRSigningDetails { - QRState: IQRState; + pendingScanRequest: QrScanRequest; successCallback?: () => void; failureCallback?: (error: string) => void; cancelCallback?: () => void; @@ -117,7 +117,7 @@ const createStyles = (colors: any) => }); const QRSigningDetails = ({ - QRState, + pendingScanRequest, successCallback, failureCallback, cancelCallback, @@ -228,11 +228,11 @@ const QRSigningDetails = ({ const buffer = signature.getRequestId(); if (buffer) { const requestId = uuidStringify(buffer); - if (QRState.sign.request?.requestId === requestId) { - KeyringController.submitQRSignature( - QRState.sign.request?.requestId as string, - ur.cbor.toString('hex'), - ); + if (pendingScanRequest?.request?.requestId === requestId) { + Engine.resolveQrKeyringScanRequest({ + type: ur.type, + cbor: ur.cbor.toString('hex'), + }); setSentOrCanceled(true); successCallback?.(); return; @@ -250,8 +250,7 @@ const QRSigningDetails = ({ failureCallback?.(strings('transaction.mismatched_qr_request_id')); }, [ - KeyringController, - QRState.sign.request?.requestId, + pendingScanRequest?.request?.requestId, failureCallback, successCallback, trackEvent, @@ -287,7 +286,7 @@ const QRSigningDetails = ({ return ( - {QRState?.sign?.request && ( + {pendingScanRequest?.request && ( void; onCancel?: () => void; onFailure?: (error: string) => void; @@ -37,7 +37,7 @@ const createStyles = (colors: any) => const QRSigningModal = ({ isVisible, - QRState, + pendingScanRequest, onSuccess, onCancel, onFailure, @@ -85,7 +85,7 @@ const QRSigningModal = ({ > , @@ -12,43 +13,18 @@ const withQRHardwareAwareness = ( // TODO: Replace "any" with type // eslint-disable-next-line @typescript-eslint/no-explicit-any const QRHardwareAwareness = (props: any) => { - const [QRState, SetQRState] = useState({ - sync: { - reading: false, - }, - sign: {}, - }); - - // TODO: Replace "any" with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const subscribeKeyringState = (value: any) => { - SetQRState(value); - }; - - useEffect(() => { - // This ensures that a QR keyring gets created if it doesn't already exist. - // This is intentionally not awaited (the subscription still gets setup correctly if called - // before the keyring is created). - // TODO: Stop automatically creating keyrings - Engine.context.KeyringController.getOrAddQRKeyring(); - Engine.controllerMessenger.subscribe( - 'KeyringController:qrKeyringStateChange', - subscribeKeyringState, - ); - return () => { - Engine.controllerMessenger.unsubscribe( - 'KeyringController:qrKeyringStateChange', - subscribeKeyringState, - ); - }; - }, []); + const { pendingScanRequest } = useSelector( + (state: RootState) => state.qrKeyringScanner, + ); return ( ); }; diff --git a/app/components/Views/AccountActions/AccountActions.tsx b/app/components/Views/AccountActions/AccountActions.tsx index 403410a076a8..70fd61865dae 100644 --- a/app/components/Views/AccountActions/AccountActions.tsx +++ b/app/components/Views/AccountActions/AccountActions.tsx @@ -47,6 +47,7 @@ import { useEIP7702Networks } from '../confirmations/hooks/7702/useEIP7702Networ import { isEvmAccountType } from '@metamask/keyring-api'; import { toHex } from '@metamask/controller-utils'; import { getMultichainBlockExplorer } from '../../../core/Multichain/networks'; +import { forgetQrDevice } from 'app/core/QrKeyring/QrKeyring'; interface AccountActionsParams { selectedAccount: InternalAccount; @@ -308,7 +309,7 @@ const AccountActions = () => { ); break; case ExtendedKeyringTypes.qr: - await controllers.KeyringController.forgetQRDevice(); + await forgetQrDevice(); trackEvent( createEventBuilder(MetaMetricsEvents.HARDWARE_WALLET_FORGOTTEN) .addProperties({ @@ -429,8 +430,8 @@ const AccountActions = () => { ) ///: END:ONLY_INCLUDE_IF } - {(networkSupporting7702Present && - !isHardwareAccount(selectedAddress)) && ( + {networkSupporting7702Present && + !isHardwareAccount(selectedAddress) && ( }, }); -/** - * Initiate a QR hardware wallet connection - * - * This returns a tuple containing a set of QR interactions, followed by a Promise representing - * the QR connection process overall. - * - * The QR interactions are returned here to ensure that we get the correct references for the - * specific keyring instance we initiated the scan with. This is to ensure that we always resolve - * the `connectQRHardware` Promise. There are equivalent methods on the `KeyringController` class - * that we could use (e.g. `KeyringController.cancelQRSynchronization` would call - * `keyring.cancelSync` for us), but these methods are unsafe to use because they might end up - * calling the method on the wrong keyring instance (e.g. if the user had locked and unlocked the - * app since initiating the scan). They will be deprecated in a future `KeyringController` version. - * - * TODO: Refactor the QR Keyring to separate interaction methods from keyring operations, so that - * interactions are not affected by our keyring operation locks and by our lock/unlock operations. - * - * @param page - The page of accounts to request (either 0, 1, or -1), for "first page", - * "next page", and "previous page" respectively. - * @returns A tuple of QR keyring interactions, and a Promise representing the QR hardware wallet - * connection process. - */ -async function initiateQRHardwareConnection( - page: 0 | 1 | -1, -): Promise< - [ - Pick, - ReturnType< - (typeof Engine)['context']['KeyringController']['connectQRHardware'] - >, - ] -> { - const KeyringController = Engine.context.KeyringController; - - const qrInteractions = await KeyringController.withKeyring( - { type: KeyringTypes.qr }, - // @ts-expect-error The QR Keyring type is not compatible with our keyring type yet - async ({ keyring }: QRKeyring) => ({ - cancelSync: keyring.cancelSync.bind(keyring), - submitCryptoAccount: keyring.submitCryptoAccount.bind(keyring), - submitCryptoHDKey: keyring.submitCryptoHDKey.bind(keyring), - }), - { createIfMissing: true }, - ); - - const connectQRHardwarePromise = KeyringController.connectQRHardware(page); - - return [qrInteractions, connectQRHardwarePromise]; -} - const ConnectQRHardware = ({ navigation }: IConnectQRHardwareProps) => { const { colors } = useTheme(); const { trackEvent, createEventBuilder } = useMetrics(); const styles = createStyles(colors); - const KeyringController = useMemo(() => { - // TODO: Replace "any" with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const { KeyringController: keyring } = Engine.context as any; - return keyring; - }, []); + const [isScanning, setIsScanning] = useState(false); + + const KeyringController = useMemo(() => Engine.context.KeyringController, []); - const [QRState, setQRState] = useState({ - sync: { - reading: false, - }, - }); - const [scannerVisible, setScannerVisible] = useState(false); const [blockingModalVisible, setBlockingModalVisible] = useState(false); const [accounts, setAccounts] = useState< { address: string; index: number; balance: string }[] @@ -160,66 +101,26 @@ const ConnectQRHardware = ({ navigation }: IConnectQRHardwareProps) => { const [existingAccounts, setExistingAccounts] = useState([]); - const qrInteractionsRef = - useRef< - Pick< - QRKeyring, - 'cancelSync' | 'submitCryptoAccount' | 'submitCryptoHDKey' - > - >(); - - const showScanner = useCallback(() => { - setScannerVisible(true); - }, []); - - const hideScanner = useCallback(() => { - setScannerVisible(false); - }, []); - - const hideModal = useCallback(() => { - qrInteractionsRef.current?.cancelSync(); - hideScanner(); - }, [hideScanner]); - useEffect(() => { KeyringController.getAccounts().then((value: string[]) => { setExistingAccounts(value); }); }, [KeyringController]); - // TODO: Replace "any" with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const subscribeKeyringState = useCallback((storeValue: any) => { - setQRState(storeValue); - }, []); - useEffect(() => { - // This ensures that a QR keyring gets created if it doesn't already exist. - // This is intentionally not awaited (the subscription still gets setup correctly if called - // before the keyring is created). - // TODO: Stop automatically creating keyrings - Engine.context.KeyringController.getOrAddQRKeyring(); - Engine.controllerMessenger.subscribe( - 'KeyringController:qrKeyringStateChange', - subscribeKeyringState, + KeyringController.withKeyring( + { type: KeyringTypes.qr }, + async (_opt: { keyring: QrKeyring }) => { + // This ensures that a QR keyring gets created if it doesn't already exist. + // This is intentionally not awaited (the subscription still gets setup correctly if called + // before the keyring is created). + // TODO: Stop automatically creating keyrings + }, + { createIfMissing: true }, ); - return () => { - Engine.controllerMessenger.unsubscribe( - 'KeyringController:qrKeyringStateChange', - subscribeKeyringState, - ); - }; - }, [KeyringController, subscribeKeyringState]); - - useEffect(() => { - if (QRState.sync.reading) { - showScanner(); - } else { - hideScanner(); - } - }, [QRState.sync, hideScanner, showScanner]); + }, [KeyringController]); - const onConnectHardware = useCallback(async () => { + const onConnectHardware = async () => { trackEvent( createEventBuilder(MetaMetricsEvents.CONTINUE_QR_HARDWARE_WALLET) .addProperties({ @@ -228,19 +129,25 @@ const ConnectQRHardware = ({ navigation }: IConnectQRHardwareProps) => { .build(), ); resetError(); - const [qrInteractions, connectQRHardwarePromise] = - await initiateQRHardwareConnection(PAGINATION_OPERATIONS.GET_FIRST_PAGE); - - qrInteractionsRef.current = qrInteractions; - const firstPageAccounts = await connectQRHardwarePromise; - delete qrInteractionsRef.current; - - setAccounts(firstPageAccounts); - }, [resetError, trackEvent, createEventBuilder]); + Logger.log('Fetching first accounts page from QR Keyring'); + try { + setIsScanning(true); + const firstAccountsPage = await withQrKeyring(({ keyring }) => + keyring.getFirstPage(), + ); + setIsScanning(false); + Logger.log('First accounts page', firstAccountsPage); + setAccounts( + // TODO: Add `balance` to the QR Keyring accounts or remove it from the expected type + firstAccountsPage.map((account) => ({ ...account, balance: '0x0' })), + ); + } catch (error) { + Logger.log('Error fetching first accounts page from QR Keyring', error); + } + }; const onScanSuccess = useCallback( - (ur: UR) => { - hideScanner(); + async (ur: UR) => { trackEvent( createEventBuilder(MetaMetricsEvents.CONNECT_HARDWARE_WALLET_SUCCESS) .addProperties({ @@ -248,56 +155,52 @@ const ConnectQRHardware = ({ navigation }: IConnectQRHardwareProps) => { }) .build(), ); - if (!qrInteractionsRef.current) { - const errorMessage = 'Missing QR keyring interactions'; - setErrorMsg(errorMessage); - throw new Error(errorMessage); - } - if (ur.type === SUPPORTED_UR_TYPE.CRYPTO_HDKEY) { - qrInteractionsRef.current.submitCryptoHDKey(ur.cbor.toString('hex')); - } else { - qrInteractionsRef.current.submitCryptoAccount(ur.cbor.toString('hex')); + try { + Engine.resolveQrKeyringScanRequest({ + type: ur.type, + cbor: ur.cbor.toString('hex'), + }); + Logger.log('QR hardware wallet connected successfully'); + resetError(); + } catch (e) { + Logger.log('Error: Connecting QR hardware wallet', e); } - resetError(); }, - [hideScanner, resetError, trackEvent, createEventBuilder], + [resetError, trackEvent, createEventBuilder], ); - const onScanError = useCallback( - async (error: string) => { - hideScanner(); - setErrorMsg(error); - if (qrInteractionsRef.current) { - qrInteractionsRef.current.cancelSync(); - } - }, - [hideScanner], - ); + const onScanError = useCallback(async (error: string) => { + setErrorMsg(error); + Engine.rejectQrKeyringScanRequest(new Error(error)); + }, []); + + const cancelScan = useCallback(() => { + Engine.rejectQrKeyringScanRequest(new Error('Scan cancelled')); + }, []); const nextPage = useCallback(async () => { resetError(); - const [qrInteractions, connectQRHardwarePromise] = - await initiateQRHardwareConnection(PAGINATION_OPERATIONS.GET_NEXT_PAGE); - - qrInteractionsRef.current = qrInteractions; - const nextPageAccounts = await connectQRHardwarePromise; - delete qrInteractionsRef.current; - - setAccounts(nextPageAccounts); + const nextAccountsPage = await withQrKeyring(async ({ keyring }) => + keyring.getNextPage(), + ); + setAccounts( + // TODO: Add `balance` to the QR Keyring accounts or remove it from the expected type + nextAccountsPage.map((account) => ({ ...account, balance: '0x0' })), + ); }, [resetError]); const prevPage = useCallback(async () => { resetError(); - const [qrInteractions, connectQRHardwarePromise] = - await initiateQRHardwareConnection( - PAGINATION_OPERATIONS.GET_PREVIOUS_PAGE, - ); - - qrInteractionsRef.current = qrInteractions; - const previousPageAccounts = await connectQRHardwarePromise; - delete qrInteractionsRef.current; - - setAccounts(previousPageAccounts); + const previousAccountsPage = await withQrKeyring(async ({ keyring }) => + keyring.getPreviousPage(), + ); + setAccounts( + // TODO: Add `balance` to the QR Keyring accounts or remove it from the expected type + previousAccountsPage.map((account) => ({ + ...account, + balance: '0x0', + })), + ); }, [resetError]); const onCheck = useCallback(() => { @@ -309,27 +212,32 @@ const ConnectQRHardware = ({ navigation }: IConnectQRHardwareProps) => { resetError(); setBlockingModalVisible(true); try { - for (const index of accountIndexs) { - await KeyringController.unlockQRHardwareWalletAccount(index); - } + await withQrKeyring(async ({ keyring }) => { + for (const index of accountIndexs) { + keyring.setAccountToUnlock(index); + await keyring.addAccounts(1); + } + }); } catch (err) { Logger.log('Error: Connecting QR hardware wallet', err); } setBlockingModalVisible(false); navigation.pop(2); }, - [KeyringController, navigation, resetError], + [navigation, resetError], ); const onForget = useCallback(async () => { resetError(); // removedAccounts and remainingAccounts are not checksummed here. - const { removedAccounts, remainingAccounts } = - await KeyringController.forgetQRDevice(); + const removedAccounts = await withQrKeyring(async ({ keyring }) => { + const existingQrAccounts = await keyring.getAccounts(); + await keyring.forgetDevice(); + return existingQrAccounts; + }); + const remainingAccounts = await KeyringController.getAccounts(); Engine.setSelectedAddress(remainingAccounts[remainingAccounts.length - 1]); - const checksummedRemovedAccounts = removedAccounts.map( - safeToChecksumAddress, - ); + const checksummedRemovedAccounts = removedAccounts.map(getChecksumAddress); removeAccountsFromPermissions(checksummedRemovedAccounts); navigation.pop(2); }, [KeyringController, navigation, resetError]); @@ -380,11 +288,11 @@ const ConnectQRHardware = ({ navigation }: IConnectQRHardwareProps) => { )} {strings('common.please_wait')} diff --git a/app/components/Views/confirmations/components/qr-info/qr-info.tsx b/app/components/Views/confirmations/components/qr-info/qr-info.tsx index caa425933b58..93e8d1964194 100644 --- a/app/components/Views/confirmations/components/qr-info/qr-info.tsx +++ b/app/components/Views/confirmations/components/qr-info/qr-info.tsx @@ -10,18 +10,16 @@ import Text from '../../../../../component-library/components/Texts/Text'; import AnimatedQRCode from '../../../../UI/QRHardware/AnimatedQRCode'; import AnimatedQRScannerModal from '../../../../UI/QRHardware/AnimatedQRScanner'; import Alert, { AlertType } from '../../../../Base/Alert'; -import { - MetaMetricsEvents, - useMetrics, -} from '../../../../hooks/useMetrics'; +import { MetaMetricsEvents, useMetrics } from '../../../../hooks/useMetrics'; import { useStyles } from '../../../../hooks/useStyles'; import { useQRHardwareContext } from '../../context/qr-hardware-context'; import { ConfirmationInfoComponentIDs } from '../../constants/info-ids'; import styleSheet from './qr-info.styles'; +import { QrScanRequestType } from '@metamask/eth-qr-keyring'; const QRInfo = () => { const { - QRState, + pendingScanRequest, cameraError, scannerVisible, setRequestCompleted, @@ -32,8 +30,6 @@ const QRInfo = () => { const [errorMessage, setErrorMessage] = useState(); const [shouldPause, setShouldPause] = useState(false); - const KeyringController = Engine.context.KeyringController; - useEffect(() => { if (scannerVisible) { setErrorMessage(undefined); @@ -47,11 +43,11 @@ const QRInfo = () => { const buffer = signature.getRequestId(); if (buffer) { const requestId = uuidStringify(buffer); - if (QRState?.sign?.request?.requestId === requestId) { - KeyringController.submitQRSignature( - QRState.sign.request?.requestId as string, - ur.cbor.toString('hex'), - ); + if (pendingScanRequest?.request?.requestId === requestId) { + Engine.resolveQrKeyringScanRequest({ + type: ur.type, + cbor: ur.cbor.toString('hex'), + }); setRequestCompleted(); return; } @@ -67,11 +63,10 @@ const QRInfo = () => { setErrorMessage(strings('transaction.mismatched_qr_request_id')); }, [ - QRState?.sign?.request?.requestId, + pendingScanRequest?.request?.requestId, createEventBuilder, setRequestCompleted, setScannerVisible, - KeyringController, trackEvent, ], ); @@ -86,41 +81,42 @@ const QRInfo = () => { return ( - {QRState?.sign?.request && ( - - - {/* todo: to be replaced by alert system */} - {errorMessage && ( - setErrorMessage(undefined)} - style={styles.alert} - > - {errorMessage} - - )} - {cameraError && ( - - {cameraError} - - )} - - - {strings('confirm.qr_scan_text')} - - - - - - )} + {pendingScanRequest?.type === QrScanRequestType.SIGN && + pendingScanRequest?.request && ( + + + {/* todo: to be replaced by alert system */} + {errorMessage && ( + setErrorMessage(undefined)} + style={styles.alert} + > + {errorMessage} + + )} + {cameraError && ( + + {cameraError} + + )} + + + {strings('confirm.qr_scan_text')} + + + + + + )} ({ context: { @@ -42,20 +42,18 @@ jest.mock('@react-navigation/native', () => ({ }), })); -const mockQRState = { - sign: { - request: { - requestId: 'c95ecc76-d6e9-4a0a-afa3-31429bc80566', - payload: { - type: 'eth-sign-request', - cbor: 'a501d82550c95ecc76d6e94a0aafa331429bc8056602581f4578616d706c652060706572736f6e616c5f7369676e60206d657373616765030305d90130a2018a182cf5183cf500f500f400f4021a73eadf6d0654126f6e36f2fbc44016d788c91b82ab4c50f74e17', - }, - title: 'Scan with your Keystone', - description: - 'After your Keystone has signed this message, click on "Scan Keystone" to receive the signature', +const mockPendingScanRequest: QrScanRequest = { + request: { + requestId: 'c95ecc76-d6e9-4a0a-afa3-31429bc80566', + payload: { + type: 'eth-sign-request', + cbor: 'a501d82550c95ecc76d6e94a0aafa331429bc8056602581f4578616d706c652060706572736f6e616c5f7369676e60206d657373616765030305d90130a2018a182cf5183cf500f500f400f4021a73eadf6d0654126f6e36f2fbc44016d788c91b82ab4c50f74e17', }, + requestTitle: 'Scan with your Keystone', + requestDescription: + 'After your Keystone has signed this message, click on "Scan Keystone" to receive the signature', }, - sync: { reading: false }, + type: QrScanRequestType.SIGN, }; describe('QRHardwareContext', () => { @@ -69,7 +67,7 @@ describe('QRHardwareContext', () => { const createQRHardwareAwarenessSpy = (mockedValues: { isQRSigningInProgress: boolean; isSigningQRObject: boolean; - QRState: IQRState; + pendingScanRequest: QrScanRequest; }) => { jest .spyOn(QRHardwareAwareness, 'useQRHardwareAwareness') @@ -81,7 +79,7 @@ describe('QRHardwareContext', () => { createQRHardwareAwarenessSpy({ isQRSigningInProgress: true, isSigningQRObject: true, - QRState: mockQRState, + pendingScanRequest: mockPendingScanRequest, }); const { getByTestId } = renderWithProvider( @@ -101,7 +99,7 @@ describe('QRHardwareContext', () => { createQRHardwareAwarenessSpy({ isQRSigningInProgress: false, isSigningQRObject: true, - QRState: mockQRState, + pendingScanRequest: mockPendingScanRequest, }); const { getByText } = renderWithProvider( @@ -112,9 +110,7 @@ describe('QRHardwareContext', () => { }, ); await userEvent.press(getByText('Cancel')); - expect( - Engine.context.KeyringController.cancelQRSignRequest, - ).toHaveBeenCalledTimes(0); + expect(Engine.rejectQrKeyringScanRequest).toHaveBeenCalledTimes(0); }); it('invokes KeyringController.cancelQRSignRequest when request is cancelled', async () => { @@ -122,7 +118,7 @@ describe('QRHardwareContext', () => { createQRHardwareAwarenessSpy({ isQRSigningInProgress: true, isSigningQRObject: true, - QRState: mockQRState, + pendingScanRequest: mockPendingScanRequest, }); const { getByText } = renderWithProvider( @@ -133,9 +129,7 @@ describe('QRHardwareContext', () => { }, ); await userEvent.press(getByText('Cancel')); - expect( - Engine.context.KeyringController.cancelQRSignRequest, - ).toHaveBeenCalledTimes(1); + expect(Engine.rejectQrKeyringScanRequest).toHaveBeenCalledTimes(1); }); it('passes correct value of QRState components', () => { @@ -143,7 +137,7 @@ describe('QRHardwareContext', () => { createQRHardwareAwarenessSpy({ isQRSigningInProgress: true, isSigningQRObject: true, - QRState: mockQRState, + pendingScanRequest: mockPendingScanRequest, }); const { getByText } = renderWithProvider( @@ -161,7 +155,7 @@ describe('QRHardwareContext', () => { createQRHardwareAwarenessSpy({ isQRSigningInProgress: true, isSigningQRObject: true, - QRState: mockQRState, + pendingScanRequest: mockPendingScanRequest, }); const { getByText } = renderWithProvider( diff --git a/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.tsx b/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.tsx index e408af0379af..5e6514770ee7 100644 --- a/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.tsx +++ b/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.tsx @@ -9,12 +9,12 @@ import React, { import { useNavigation, NavigationAction } from '@react-navigation/native'; import Engine from '../../../../../core/Engine'; -import { IQRState } from '../../../../UI/QRHardware/types'; import { useCamera } from './useCamera'; import { useQRHardwareAwareness } from './useQRHardwareAwareness'; +import { QrScanRequest } from '@metamask/eth-qr-keyring'; export interface QRHardwareContextType { - QRState?: IQRState; + pendingScanRequest?: QrScanRequest; cameraError: string | undefined; cancelQRScanRequestIfPresent: () => Promise; isQRSigningInProgress: boolean; @@ -26,7 +26,7 @@ export interface QRHardwareContextType { } export const QRHardwareContext = createContext({ - QRState: undefined, + pendingScanRequest: undefined, cameraError: undefined, cancelQRScanRequestIfPresent: () => Promise.resolve(), isQRSigningInProgress: false, @@ -41,25 +41,22 @@ export const QRHardwareContextProvider: React.FC<{ children: ReactElement[] | ReactElement; }> = ({ children }) => { const navigation = useNavigation(); - const { isQRSigningInProgress, isSigningQRObject, QRState } = + const { isQRSigningInProgress, isSigningQRObject, pendingScanRequest } = useQRHardwareAwareness(); const { cameraError, hasCameraPermission } = useCamera(isSigningQRObject); const [scannerVisible, setScannerVisible] = useState(false); const [isRequestCompleted, setRequestCompleted] = useState(false); - const KeyringController = Engine.context.KeyringController; - const cancelRequest = useCallback( (e: { preventDefault: () => void; data: { action: NavigationAction } }) => { if (isRequestCompleted) { return; } e.preventDefault(); - KeyringController.cancelQRSignRequest().then(() => { - navigation.dispatch(e.data.action); - }); + Engine.rejectQrKeyringScanRequest(new Error('Request cancelled')); + navigation.dispatch(e.data.action); }, - [KeyringController, isRequestCompleted, navigation], + [isRequestCompleted, navigation], ); useEffect(() => { @@ -71,20 +68,15 @@ export const QRHardwareContextProvider: React.FC<{ if (!isQRSigningInProgress) { return; } - await KeyringController.cancelQRSignRequest(); + Engine.rejectQrKeyringScanRequest(new Error('Request cancelled')); setRequestCompleted(true); setScannerVisible(false); - }, [ - KeyringController, - isQRSigningInProgress, - setRequestCompleted, - setScannerVisible, - ]); + }, [isQRSigningInProgress, setRequestCompleted, setScannerVisible]); return ( ({ - context: { - KeyringController: { - getOrAddQRKeyring: jest.fn(), - }, - }, - controllerMessenger: { - subscribe: jest.fn(), - unsubscribe: jest.fn(), - }, -})); const initialState = { - QRState: { - sync: { - reading: false, - }, - sign: {}, - }, + pendingScanRequest: null, isQRSigningInProgress: false, isSigningQRObject: false, }; describe('useQRHardwareAwareness', () => { - it('invokes required methods from Engine', () => { - renderHook(() => useQRHardwareAwareness()); - expect( - Engine.context.KeyringController.getOrAddQRKeyring, - ).toHaveBeenCalledTimes(1); - expect(Engine.controllerMessenger.subscribe).toHaveBeenCalledTimes(1); - }); it('returns correct initial values', () => { const { result } = renderHook(() => useQRHardwareAwareness()); expect(result.current).toMatchObject(initialState); diff --git a/app/components/Views/confirmations/context/qr-hardware-context/useQRHardwareAwareness.ts b/app/components/Views/confirmations/context/qr-hardware-context/useQRHardwareAwareness.ts index e672a6a07f34..d558d495af48 100644 --- a/app/components/Views/confirmations/context/qr-hardware-context/useQRHardwareAwareness.ts +++ b/app/components/Views/confirmations/context/qr-hardware-context/useQRHardwareAwareness.ts @@ -1,43 +1,15 @@ -import { useEffect, useState } from 'react'; - -import Engine from '../../../../../core/Engine'; -import { IQRState } from '../../../../UI/QRHardware/types'; +import { QrScanRequestType } from '@metamask/eth-qr-keyring'; +import { RootState } from 'app/components/UI/BasicFunctionality/BasicFunctionalityModal/BasicFunctionalityModal.test'; +import { useSelector } from 'react-redux'; export const useQRHardwareAwareness = () => { - const [QRState, SetQRState] = useState({ - sync: { - reading: false, - }, - sign: {}, - }); - - // TODO: Replace "any" with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const subscribeKeyringState = (value: any) => { - SetQRState(value); - }; - - useEffect(() => { - // This ensures that a QR keyring gets created if it doesn't already exist. - // This is intentionally not awaited (the subscription still gets setup correctly if called - // before the keyring is created). - // TODO: Stop automatically creating keyrings - Engine.context.KeyringController.getOrAddQRKeyring(); - Engine.controllerMessenger.subscribe( - 'KeyringController:qrKeyringStateChange', - subscribeKeyringState, - ); - return () => { - Engine.controllerMessenger.unsubscribe( - 'KeyringController:qrKeyringStateChange', - subscribeKeyringState, - ); - }; - }, []); + const { pendingScanRequest } = useSelector( + (state: RootState) => state.qrKeyringScanner, + ); return { - isQRSigningInProgress: QRState?.sign?.request !== undefined, - isSigningQRObject: !!QRState?.sign?.request, - QRState, + isQRSigningInProgress: pendingScanRequest?.type === QrScanRequestType.SIGN, + isSigningQRObject: pendingScanRequest?.type === QrScanRequestType.SIGN, + pendingScanRequest, }; }; diff --git a/app/core/Engine/Engine.ts b/app/core/Engine/Engine.ts index 3476dde3ff64..cd2d8fb7bf58 100644 --- a/app/core/Engine/Engine.ts +++ b/app/core/Engine/Engine.ts @@ -60,7 +60,12 @@ import { } from '@metamask/snaps-rpc-methods'; import type { EnumToUnion, DialogType } from '@metamask/snaps-sdk'; ///: END:ONLY_INCLUDE_IF -import { MetaMaskKeyring as QRHardwareKeyring } from '@keystonehq/metamask-airgapped-keyring'; +import { + QrKeyring, + QrKeyringScannerBridge, + QrScanRequest, + SerializedUR, +} from '@metamask/eth-qr-keyring'; import { LoggingController } from '@metamask/logging-controller'; import { TokenSearchDiscoveryControllerMessenger } from '@metamask/token-search-discovery-controller'; import { @@ -110,7 +115,12 @@ import { unrestrictedMethods, } from '../Permissions/specifications.js'; import { backupVault } from '../BackupVault'; -import { Hex, Json } from '@metamask/utils'; +import { + DeferredPromise, + Hex, + Json, + createDeferredPromise, +} from '@metamask/utils'; import { providerErrors } from '@metamask/rpc-errors'; import { PPOM, ppomInit } from '../../lib/ppom/PPOMView'; @@ -231,6 +241,7 @@ import { captureException } from '@sentry/react-native'; import { WebSocketServiceInit } from './controllers/snaps/websocket-service-init'; import { seedlessOnboardingControllerInit } from './controllers/seedless-onboarding-controller'; +import { scanCompleted, scanRequested } from '../redux/slices/qrKeyringScanner'; const NON_EMPTY = 'NON_EMPTY'; @@ -281,6 +292,9 @@ export class Engine { smartTransactionsController: SmartTransactionsController; transactionController: TransactionController; multichainRouter: MultichainRouter; + + pendingQrKeyringScanRequest?: DeferredPromise; + /** * Creates a CoreController instance */ @@ -551,13 +565,16 @@ export class Engine { const additionalKeyrings = []; + const qrKeyringBridge = new QrKeyringScannerBridge({ + requestScan: (request) => this.requestQrKeyringScan(request), + }); const qrKeyringBuilder = () => { - const keyring = new QRHardwareKeyring(); + const keyring = new QrKeyring({ bridge: qrKeyringBridge }); // to fix the bug in #9560, forgetDevice will reset all keyring properties to default. keyring.forgetDevice(); return keyring; }; - qrKeyringBuilder.type = QRHardwareKeyring.type; + qrKeyringBuilder.type = QrKeyring.type; additionalKeyrings.push(qrKeyringBuilder); @@ -2251,6 +2268,37 @@ export class Engine { } } + async requestQrKeyringScan(request: QrScanRequest): Promise { + if (this.pendingQrKeyringScanRequest) { + throw new Error( + 'A QR keyring scan request is already pending. Please wait for it to complete before requesting a new scan.', + ); + } + this.pendingQrKeyringScanRequest = createDeferredPromise({ + suppressUnhandledRejection: true, + }); + store.dispatch(scanRequested(request)); + return this.pendingQrKeyringScanRequest.promise; + } + + resolveQrKeyringScanRequest(response: SerializedUR) { + if (!this.pendingQrKeyringScanRequest) { + throw new Error('No pending QR keyring scan request to resolve.'); + } + this.pendingQrKeyringScanRequest.resolve(response); + this.pendingQrKeyringScanRequest = undefined; + store.dispatch(scanCompleted()); + } + + rejectQrKeyringScanRequest(error: Error) { + if (!this.pendingQrKeyringScanRequest) { + throw new Error('No pending QR keyring scan request to reject.'); + } + this.pendingQrKeyringScanRequest.reject(error); + this.pendingQrKeyringScanRequest = undefined; + store.dispatch(scanCompleted()); + } + // This should be used instead of directly calling PreferencesController.setSelectedAddress or AccountsController.setSelectedAccount setSelectedAccount(address: string) { const { AccountsController, PreferencesController } = this.context; @@ -2467,6 +2515,16 @@ export default { } = {}, ) => instance?.rejectPendingApproval(id, reason, opts), + resolveQrKeyringScanRequest: (response: SerializedUR) => { + assertEngineExists(instance); + instance.resolveQrKeyringScanRequest(response); + }, + + rejectQrKeyringScanRequest: (error: Error) => { + assertEngineExists(instance); + instance.rejectQrKeyringScanRequest(error); + }, + setSelectedAddress: (address: string) => { assertEngineExists(instance); instance.setSelectedAccount(address); diff --git a/app/core/QrKeyring/QrKeyring.ts b/app/core/QrKeyring/QrKeyring.ts new file mode 100644 index 000000000000..ed189353b080 --- /dev/null +++ b/app/core/QrKeyring/QrKeyring.ts @@ -0,0 +1,34 @@ +import { QrKeyring } from '@metamask/eth-qr-keyring'; +import { KeyringMetadata } from '@metamask/keyring-controller'; +import Engine from '../Engine'; +import ExtendedKeyringTypes from 'app/constants/keyringTypes'; + +/** + * Perform an operation with the QR keyring. + * + * If no QR keyring is found, one is created. + * + * Note that the `operation` function should only be used for interactions with the keyring. + * If you call KeyringController methods within this function, it could result in a deadlock. + * + * @param operation - The keyring operation to perform. + * @returns The stored Qr Keyring + * @throws If there is no QR keyring available or if the operation fails. + */ +export const withQrKeyring = async ( + operation: (selectedKeyring: { + keyring: QrKeyring; + metadata: KeyringMetadata; + }) => Promise, +): Promise => + Engine.context.KeyringController.withKeyring( + { type: ExtendedKeyringTypes.qr, index: 0 }, + operation, + ); + +/** + * Forget the QR keyring device, removing all accounts and + * paired device information. + */ +export const forgetQrDevice = async () => + withQrKeyring(async ({ keyring }) => keyring.forgetDevice()); diff --git a/app/core/redux/slices/qrKeyringScanner/index.ts b/app/core/redux/slices/qrKeyringScanner/index.ts new file mode 100644 index 000000000000..c6abbaf04f82 --- /dev/null +++ b/app/core/redux/slices/qrKeyringScanner/index.ts @@ -0,0 +1,33 @@ +import { QrScanRequest } from '@metamask/eth-qr-keyring'; +import { PayloadAction, createSlice } from '@reduxjs/toolkit'; + +export interface QrKeyringScannerState { + pendingScanRequest?: QrScanRequest; + isScanning: boolean; +} + +export const initialState: QrKeyringScannerState = { + isScanning: false, +}; + +const name = 'qrKeyringScanner'; + +const slice = createSlice({ + name, + initialState, + reducers: { + scanRequested: (state, action: PayloadAction) => { + state.pendingScanRequest = action.payload; + state.isScanning = true; + }, + scanCompleted: (state) => { + state.pendingScanRequest = undefined; + state.isScanning = false; + }, + }, +}); + +const { actions, reducer } = slice; + +export default reducer; +export const { scanRequested, scanCompleted } = actions; diff --git a/app/reducers/index.ts b/app/reducers/index.ts index d5dc278c4cdb..939c2c0c14e8 100644 --- a/app/reducers/index.ts +++ b/app/reducers/index.ts @@ -26,6 +26,7 @@ import rpcEventReducer from './rpcEvents'; import accountsReducer from './accounts'; import sdkReducer from './sdk'; import inpageProviderReducer from '../core/redux/slices/inpageProvider'; +import qrKeyringScannerReducer from '../core/redux/slices/qrKeyringScanner'; import confirmationMetricsReducer from '../core/redux/slices/confirmationMetrics'; import originThrottlingReducer from '../core/redux/slices/originThrottling'; import notificationsAccountsProvider from '../core/redux/slices/notifications'; @@ -122,6 +123,7 @@ export interface RootState { originThrottling: StateFromReducer; notifications: StateFromReducer; bridge: StateFromReducer; + qrKeyringScanner: StateFromReducer; banners: BannersState; performance?: PerformanceState; } @@ -160,6 +162,7 @@ const baseReducers = { bridge: bridgeReducer, banners: bannersReducer, confirmationMetrics: confirmationMetricsReducer, + qrKeyringScanner: qrKeyringScannerReducer, }; if (isTest) { diff --git a/app/util/test/initial-root-state.ts b/app/util/test/initial-root-state.ts index a9ad53fd6a79..a5dc65f5f73a 100644 --- a/app/util/test/initial-root-state.ts +++ b/app/util/test/initial-root-state.ts @@ -6,6 +6,7 @@ import { initialState as initialInpageProvider } from '../../core/redux/slices/i import { initialState as confirmationMetrics } from '../../core/redux/slices/confirmationMetrics'; import { initialState as originThrottling } from '../../core/redux/slices/originThrottling'; import { initialState as initialBridgeState } from '../../core/redux/slices/bridge'; +import { initialState as initialQrKeyringScannerState } from '../../core/redux/slices/qrKeyringScanner'; import initialBackgroundState from './initial-background-state.json'; import { userInitialState } from '../../reducers/user'; import { initialNavigationState } from '../../reducers/navigation'; @@ -39,6 +40,7 @@ const initialRootState: RootState = { networkOnboarded: undefined, security: initialSecurityState, signatureRequest: undefined, + qrKeyringScanner: initialQrKeyringScannerState, sdk: { connections: {}, approvedHosts: {}, diff --git a/package.json b/package.json index 51eb9067b5ad..6232068d39f8 100644 --- a/package.json +++ b/package.json @@ -153,13 +153,13 @@ "**/@ethersproject/signing-key/elliptic": "^6.6.1", "**/@walletconnect/utils/elliptic": "^6.6.1", "@metamask/keyring-controller/@ethereumjs/tx": "npm:@ethereumjs/tx@5.4.0", - "@keystonehq/metamask-airgapped-keyring": "^0.15.2", "metro/image-size": "^1.2.1", "content-hash/**/base-x": "3.0.11", "multihashes/**/base-x": "3.0.11", "@keystonehq/ur-decoder/**/base-x": "3.0.11", "@ethereumjs/tx": "^5.4.0", - "@metamask/controller-utils": "npm:@metamask/controller-utils@^11.11.0" + "@metamask/controller-utils": "npm:@metamask/controller-utils@^11.11.0", + "@metamask/keyring-controller": "npm:@metamask-previews/keyring-controller@22.0.2-preview-d7c00b1" }, "dependencies": { "@config-plugins/detox": "^9.0.0", @@ -167,7 +167,6 @@ "@consensys/on-ramp-sdk": "2.1.10", "@ethersproject/abi": "^5.7.0", "@keystonehq/bc-ur-registry-eth": "^0.21.0", - "@keystonehq/metamask-airgapped-keyring": "^0.15.2", "@keystonehq/ur-decoder": "^0.12.2", "@ledgerhq/react-native-hw-transport-ble": "^6.34.1", "@metamask/account-tree-controller": "0.1.1", @@ -203,7 +202,7 @@ "@metamask/json-rpc-middleware-stream": "^8.0.7", "@metamask/key-tree": "^10.1.1", "@metamask/keyring-api": "^18.0.0", - "@metamask/keyring-controller": "^22.0.1", + "@metamask/keyring-controller": "npm:@metamask-previews/keyring-controller@22.0.2-preview-d7c00b1", "@metamask/keyring-internal-api": "^6.2.0", "@metamask/keyring-snap-client": "^5.0.0", "@metamask/logging-controller": "^6.0.4", @@ -249,6 +248,7 @@ "@metamask/token-search-discovery-controller": "^3.1.0", "@metamask/transaction-controller": "^58.1.1", "@metamask/utils": "^11.4.2", + "@metamask/eth-qr-keyring": "../accounts/packages/keyring-eth-qr/", "@ngraveio/bc-ur": "^1.1.6", "@noble/hashes": "^1.7.1", "@notifee/react-native": "^9.0.0", diff --git a/yarn.lock b/yarn.lock index 64a8f1a5b05f..55f9eaa77837 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1908,7 +1908,7 @@ resolved "https://registry.yarnpkg.com/@ethereumjs/rlp/-/rlp-5.0.2.tgz#c89bd82f2f3bec248ab2d517ae25f5bbc4aac842" integrity sha512-DziebCdg4JpGlEqEdGgXmjqcFoJi+JGulUXwEjsZGAscAQ7MyD/7LE/GVCP29vEQxKc7AAwjT3A2ywHp2xfoCA== -"@ethereumjs/tx@5.1.0", "@ethereumjs/tx@^10.0.0", "@ethereumjs/tx@^4.2.0", "@ethereumjs/tx@^5.2.1", "@ethereumjs/tx@^5.4.0", "@ethereumjs/tx@npm:@ethereumjs/tx@5.4.0": +"@ethereumjs/tx@^10.0.0", "@ethereumjs/tx@^4.2.0", "@ethereumjs/tx@^5.2.1", "@ethereumjs/tx@^5.4.0", "@ethereumjs/tx@npm:@ethereumjs/tx@5.4.0": version "5.4.0" resolved "https://registry.yarnpkg.com/@ethereumjs/tx/-/tx-5.4.0.tgz#6f47894cc3e2d4e63d87c62b41ed7e8180a1de58" integrity sha512-SCHnK7m/AouZ7nyoR0MEXw1OO/tQojSbp88t8oxhwes5iZkZCtfFdUrJaiIb72qIpH2FVw6s1k1uP7LXuH7PsA== @@ -1918,14 +1918,6 @@ "@ethereumjs/util" "^9.1.0" ethereum-cryptography "^2.2.1" -"@ethereumjs/util@9.1.0", "@ethereumjs/util@^9.0.2", "@ethereumjs/util@^9.0.3", "@ethereumjs/util@^9.1.0": - version "9.1.0" - resolved "https://registry.yarnpkg.com/@ethereumjs/util/-/util-9.1.0.tgz#75e3898a3116d21c135fa9e29886565609129bce" - integrity sha512-XBEKsYqLGXLah9PNJbgdkigthkG7TAGvlD/sH12beMXEyHDyigfcbdvHhmLyDWgDyOJn4QwiQUaF7yeuhnjdog== - dependencies: - "@ethereumjs/rlp" "^5.0.2" - ethereum-cryptography "^2.2.1" - "@ethereumjs/util@^10.0.0": version "10.0.0" resolved "https://registry.yarnpkg.com/@ethereumjs/util/-/util-10.0.0.tgz#1afcb06d769cb979a628a65604ba7e7fb61167ac" @@ -1943,6 +1935,14 @@ ethereum-cryptography "^2.0.0" micro-ftch "^0.3.1" +"@ethereumjs/util@^9.0.2", "@ethereumjs/util@^9.0.3", "@ethereumjs/util@^9.1.0": + version "9.1.0" + resolved "https://registry.yarnpkg.com/@ethereumjs/util/-/util-9.1.0.tgz#75e3898a3116d21c135fa9e29886565609129bce" + integrity sha512-XBEKsYqLGXLah9PNJbgdkigthkG7TAGvlD/sH12beMXEyHDyigfcbdvHhmLyDWgDyOJn4QwiQUaF7yeuhnjdog== + dependencies: + "@ethereumjs/rlp" "^5.0.2" + ethereum-cryptography "^2.2.1" + "@ethersproject/abi@5.7.0", "@ethersproject/abi@^5.7.0": version "5.7.0" resolved "https://registry.yarnpkg.com/@ethersproject/abi/-/abi-5.7.0.tgz#b3f3e045bbbeed1af3947335c247ad625a44e449" @@ -4389,19 +4389,6 @@ resolved "https://registry.yarnpkg.com/@keystonehq/alias-sampling/-/alias-sampling-0.1.2.tgz#63af931ffe6500aef4c0d87775a5b279189abf8d" integrity sha512-5ukLB3bcgltgaFfQfYKYwHDUbwHicekYo53fSEa7xhVkAEqsA74kxdIwoBIURmGUtXe3EVIRm4SYlgcrt2Ri0w== -"@keystonehq/base-eth-keyring@0.15.1": - version "0.15.1" - resolved "https://registry.yarnpkg.com/@keystonehq/base-eth-keyring/-/base-eth-keyring-0.15.1.tgz#360b6df8dfdc87a3021256fe1173d9d7516ad1ae" - integrity sha512-YaHTT5tJwTR4OUpn5nc3v4pZxNSqXZH3TON736fwTEZ+iIofzFTIW4NRBQ/0rQ/S2Y785T92D+imZH/fTVhUpw== - dependencies: - "@ethereumjs/rlp" "^4.0.1" - "@ethereumjs/tx" "5.1.0" - "@ethereumjs/util" "^8.0.0" - "@keystonehq/bc-ur-registry-eth" "^0.19.1" - hdkey "^2.0.1" - rlp "^3.0.0" - uuid "^8.3.2" - "@keystonehq/bc-ur-registry-eth@^0.19.1": version "0.19.1" resolved "https://registry.yarnpkg.com/@keystonehq/bc-ur-registry-eth/-/bc-ur-registry-eth-0.19.1.tgz#eac508b9d15d17c0abd00b107691585f9c789ffc" @@ -4477,20 +4464,6 @@ bs58check "^2.1.2" tslib "^2.3.0" -"@keystonehq/metamask-airgapped-keyring@^0.14.1", "@keystonehq/metamask-airgapped-keyring@^0.15.2": - version "0.15.2" - resolved "https://registry.yarnpkg.com/@keystonehq/metamask-airgapped-keyring/-/metamask-airgapped-keyring-0.15.2.tgz#d4b8aa807458124d437b3537253d570231b82bac" - integrity sha512-csC7m+7CbWwl2Qhm41/apOAR1KOkLdjXxChfKVRR+k4+KCOMQvUnzksnymhAtnkEOiePGjpf9ZuGeJtYD4h52w== - dependencies: - "@ethereumjs/rlp" "^4.0.1" - "@ethereumjs/tx" "5.1.0" - "@ethereumjs/util" "9.1.0" - "@keystonehq/base-eth-keyring" "0.15.1" - "@keystonehq/bc-ur-registry-eth" "^0.19.1" - "@metamask/obs-store" "^9.0.0" - rlp "^2.2.6" - uuid "^8.3.2" - "@keystonehq/sdk@^0.19.2": version "0.19.2" resolved "https://registry.yarnpkg.com/@keystonehq/sdk/-/sdk-0.19.2.tgz#2c7e35b93fc8c853b1161f32c1f0f68d272b43f3" @@ -5169,6 +5142,20 @@ "@metamask/eth-sig-util" "^8.2.0" hdkey "^2.1.0" +"@metamask/eth-qr-keyring@../accounts/packages/keyring-eth-qr/": + version "0.0.0" + dependencies: + "@ethereumjs/rlp" "^5.0.2" + "@ethereumjs/tx" "^5.4.0" + "@ethereumjs/util" "^9.1.0" + "@keystonehq/bc-ur-registry-eth" "^0.19.1" + "@metamask/eth-sig-util" "^8.2.0" + "@metamask/keyring-utils" "^3.0.0" + "@metamask/utils" "^11.1.0" + async-mutex "^0.5.0" + hdkey "^2.1.0" + uuid "^9.0.1" + "@metamask/eth-query@^4.0.0": version "4.0.0" resolved "https://registry.yarnpkg.com/@metamask/eth-query/-/eth-query-4.0.0.tgz#a8c1651b69e298da58628b1c09d31dd504a939b3" @@ -5350,13 +5337,12 @@ "@metamask/utils" "^11.1.0" bitcoin-address-validation "^2.2.3" -"@metamask/keyring-controller@^22.0.1": - version "22.0.1" - resolved "https://registry.yarnpkg.com/@metamask/keyring-controller/-/keyring-controller-22.0.1.tgz#a672c97a8640bf3dc96ec316fa315cc9a50b2e7b" - integrity sha512-LJqqrOPKZOWk3zDLrEDwFHB58qn1wGX1b56aduUpO2HVnpIwWt5b8W2sNbCqqfdL5vKqDhIB0QAMIMqiGu4DxQ== +"@metamask/keyring-controller@npm:@metamask-previews/keyring-controller@22.0.2-preview-d7c00b1": + version "22.0.2-preview-d7c00b1" + resolved "https://registry.yarnpkg.com/@metamask-previews/keyring-controller/-/keyring-controller-22.0.2-preview-d7c00b1.tgz#ea42686bee4f7dd076f4dfa6cd547d40befce96c" + integrity sha512-Pbv+ry/6rAc039KEl1uovrk2VEJcng9vntZDNYxIycychP7KNAQ9Lvbq76gyQdt7/qNgjissB/qQkeLe9kT7KA== dependencies: "@ethereumjs/util" "^9.1.0" - "@keystonehq/metamask-airgapped-keyring" "^0.14.1" "@metamask/base-controller" "^8.0.1" "@metamask/browser-passworder" "^4.3.0" "@metamask/eth-hd-keyring" "^12.0.0" @@ -5610,14 +5596,6 @@ once "^1.4.0" readable-stream "^3.6.2" -"@metamask/obs-store@^9.0.0": - version "9.0.0" - resolved "https://registry.yarnpkg.com/@metamask/obs-store/-/obs-store-9.0.0.tgz#fa50c988b4635817ff0454bc9763b1cf6b37d9e9" - integrity sha512-GDsEh2DTHgmISzJt8erf9T4Ph38iwD2yDJ6J1YFq/IcWRGnT1bkgSEVqZMv9c9JloX02T5bFIUK6+9m9EycI6A== - dependencies: - "@metamask/safe-event-emitter" "^3.0.0" - readable-stream "^3.6.2" - "@metamask/permission-controller@^11.0.6": version "11.0.6" resolved "https://registry.yarnpkg.com/@metamask/permission-controller/-/permission-controller-11.0.6.tgz#4069f6ed29d514a7e897f177efc9f798bba8668a" @@ -28704,18 +28682,13 @@ ripple-keypairs@^2.0.0: "@xrplf/isomorphic" "^1.0.0" ripple-address-codec "^5.0.0" -rlp@^2.2.3, rlp@^2.2.4, rlp@^2.2.6: +rlp@^2.2.3, rlp@^2.2.4: version "2.2.7" resolved "https://registry.yarnpkg.com/rlp/-/rlp-2.2.7.tgz#33f31c4afac81124ac4b283e2bd4d9720b30beaf" integrity sha512-d5gdPmgQ0Z+AklL2NVXr/IoSjNZFfTVvQWzL/AM2AOcSzYP2xjlb0AC8YyCLc41MSNf6P6QVtjgPdmVtzb+4lQ== dependencies: bn.js "^5.2.0" -rlp@^3.0.0: - version "3.0.0" - resolved "https://registry.yarnpkg.com/rlp/-/rlp-3.0.0.tgz#5a60725ca4314a3a165feecca1836e4f2c1e2343" - integrity sha512-PD6U2PGk6Vq2spfgiWZdomLvRGDreBLxi5jv5M8EpRo3pU6VEm31KO+HFxE18Q3vgqfDrQ9pZA3FP95rkijNKw== - rpc-websockets@^9.0.2: version "9.1.1" resolved "https://registry.yarnpkg.com/rpc-websockets/-/rpc-websockets-9.1.1.tgz#5764336f3623ee1c5cc8653b7335183e3c0c78bd" From 556058a4d89a9e2c1661a2ea27f0bc0fda5423d2 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 15 Jul 2025 11:29:04 +0200 Subject: [PATCH 02/31] fix Engine frozen --- .../Views/ConnectQRHardware/index.tsx | 52 ++++++------------- app/core/Engine/Engine.ts | 25 +++++---- app/core/QrKeyring/QrKeyring.ts | 6 ++- package.json | 3 +- yarn.lock | 11 ++-- 5 files changed, 44 insertions(+), 53 deletions(-) diff --git a/app/components/Views/ConnectQRHardware/index.tsx b/app/components/Views/ConnectQRHardware/index.tsx index e65c5b158eda..dfb7e2335caf 100644 --- a/app/components/Views/ConnectQRHardware/index.tsx +++ b/app/components/Views/ConnectQRHardware/index.tsx @@ -24,12 +24,11 @@ import { fontStyles } from '../../../styles/common'; import Logger from '../../../util/Logger'; import { removeAccountsFromPermissions } from '../../../core/Permissions'; import { useMetrics } from '../../../components/hooks/useMetrics'; -import { KeyringTypes } from '@metamask/keyring-controller'; -import { HardwareDeviceTypes } from '../../../constants/keyringTypes'; +import ExtendedKeyringTypes, { + HardwareDeviceTypes, +} from '../../../constants/keyringTypes'; import { ThemeColors } from '@metamask/design-tokens'; -import { RootState } from 'app/reducers'; -import { useSelector } from 'react-redux'; -import { QrKeyring, QrScanRequestType } from '@metamask/eth-qr-keyring'; +import { QrScanRequestType } from '@metamask/eth-qr-keyring'; import { withQrKeyring } from 'app/core/QrKeyring/QrKeyring'; import { getChecksumAddress } from '@metamask/utils'; @@ -107,19 +106,6 @@ const ConnectQRHardware = ({ navigation }: IConnectQRHardwareProps) => { }); }, [KeyringController]); - useEffect(() => { - KeyringController.withKeyring( - { type: KeyringTypes.qr }, - async (_opt: { keyring: QrKeyring }) => { - // This ensures that a QR keyring gets created if it doesn't already exist. - // This is intentionally not awaited (the subscription still gets setup correctly if called - // before the keyring is created). - // TODO: Stop automatically creating keyrings - }, - { createIfMissing: true }, - ); - }, [KeyringController]); - const onConnectHardware = async () => { trackEvent( createEventBuilder(MetaMetricsEvents.CONTINUE_QR_HARDWARE_WALLET) @@ -129,25 +115,23 @@ const ConnectQRHardware = ({ navigation }: IConnectQRHardwareProps) => { .build(), ); resetError(); - Logger.log('Fetching first accounts page from QR Keyring'); try { setIsScanning(true); const firstAccountsPage = await withQrKeyring(({ keyring }) => keyring.getFirstPage(), ); - setIsScanning(false); - Logger.log('First accounts page', firstAccountsPage); setAccounts( // TODO: Add `balance` to the QR Keyring accounts or remove it from the expected type firstAccountsPage.map((account) => ({ ...account, balance: '0x0' })), ); - } catch (error) { - Logger.log('Error fetching first accounts page from QR Keyring', error); + } finally { + setIsScanning(false); } }; const onScanSuccess = useCallback( async (ur: UR) => { + setIsScanning(false); trackEvent( createEventBuilder(MetaMetricsEvents.CONNECT_HARDWARE_WALLET_SUCCESS) .addProperties({ @@ -155,16 +139,11 @@ const ConnectQRHardware = ({ navigation }: IConnectQRHardwareProps) => { }) .build(), ); - try { - Engine.resolveQrKeyringScanRequest({ - type: ur.type, - cbor: ur.cbor.toString('hex'), - }); - Logger.log('QR hardware wallet connected successfully'); - resetError(); - } catch (e) { - Logger.log('Error: Connecting QR hardware wallet', e); - } + Engine.resolveQrKeyringScanRequest({ + type: ur.type, + cbor: ur.cbor.toString('hex'), + }); + resetError(); }, [resetError, trackEvent, createEventBuilder], ); @@ -229,14 +208,15 @@ const ConnectQRHardware = ({ navigation }: IConnectQRHardwareProps) => { const onForget = useCallback(async () => { resetError(); - // removedAccounts and remainingAccounts are not checksummed here. + const remainingAccounts = KeyringController.state.keyrings + .filter((keyring) => keyring.type !== ExtendedKeyringTypes.qr) + .flatMap((keyring) => keyring.accounts); + Engine.setSelectedAddress(remainingAccounts[remainingAccounts.length - 1]); const removedAccounts = await withQrKeyring(async ({ keyring }) => { const existingQrAccounts = await keyring.getAccounts(); await keyring.forgetDevice(); return existingQrAccounts; }); - const remainingAccounts = await KeyringController.getAccounts(); - Engine.setSelectedAddress(remainingAccounts[remainingAccounts.length - 1]); const checksummedRemovedAccounts = removedAccounts.map(getChecksumAddress); removeAccountsFromPermissions(checksummedRemovedAccounts); navigation.pop(2); diff --git a/app/core/Engine/Engine.ts b/app/core/Engine/Engine.ts index cd2d8fb7bf58..a00791d4b588 100644 --- a/app/core/Engine/Engine.ts +++ b/app/core/Engine/Engine.ts @@ -293,7 +293,11 @@ export class Engine { transactionController: TransactionController; multichainRouter: MultichainRouter; - pendingQrKeyringScanRequest?: DeferredPromise; + #pendingQrKeyringScan: { + request: DeferredPromise | null; + } = { + request: null, + }; /** * Creates a CoreController instance @@ -2269,33 +2273,34 @@ export class Engine { } async requestQrKeyringScan(request: QrScanRequest): Promise { - if (this.pendingQrKeyringScanRequest) { + if (this.#pendingQrKeyringScan.request) { throw new Error( 'A QR keyring scan request is already pending. Please wait for it to complete before requesting a new scan.', ); } - this.pendingQrKeyringScanRequest = createDeferredPromise({ + const deferredPromise = createDeferredPromise({ suppressUnhandledRejection: true, }); + this.#pendingQrKeyringScan.request = deferredPromise; store.dispatch(scanRequested(request)); - return this.pendingQrKeyringScanRequest.promise; + return deferredPromise.promise; } resolveQrKeyringScanRequest(response: SerializedUR) { - if (!this.pendingQrKeyringScanRequest) { + if (!this.#pendingQrKeyringScan.request) { throw new Error('No pending QR keyring scan request to resolve.'); } - this.pendingQrKeyringScanRequest.resolve(response); - this.pendingQrKeyringScanRequest = undefined; + this.#pendingQrKeyringScan.request.resolve(response); + this.#pendingQrKeyringScan.request = null; store.dispatch(scanCompleted()); } rejectQrKeyringScanRequest(error: Error) { - if (!this.pendingQrKeyringScanRequest) { + if (!this.#pendingQrKeyringScan.request) { throw new Error('No pending QR keyring scan request to reject.'); } - this.pendingQrKeyringScanRequest.reject(error); - this.pendingQrKeyringScanRequest = undefined; + this.#pendingQrKeyringScan.request.reject(error); + this.#pendingQrKeyringScan.request = null; store.dispatch(scanCompleted()); } diff --git a/app/core/QrKeyring/QrKeyring.ts b/app/core/QrKeyring/QrKeyring.ts index ed189353b080..529c5e170ba2 100644 --- a/app/core/QrKeyring/QrKeyring.ts +++ b/app/core/QrKeyring/QrKeyring.ts @@ -22,8 +22,12 @@ export const withQrKeyring = async ( }) => Promise, ): Promise => Engine.context.KeyringController.withKeyring( - { type: ExtendedKeyringTypes.qr, index: 0 }, + { type: ExtendedKeyringTypes.qr }, operation, + // TODO: Refactor this to stop creating the keyring on-demand + // Instead create it only in response to an explicit user action, and do + // not allow Ledger interactions until after that has been done. + { createIfMissing: true }, ); /** diff --git a/package.json b/package.json index 6232068d39f8..f29e4a573c07 100644 --- a/package.json +++ b/package.json @@ -190,6 +190,7 @@ "@metamask/eth-json-rpc-filters": "^9.0.0", "@metamask/eth-json-rpc-middleware": "^17.0.1", "@metamask/eth-ledger-bridge-keyring": "11.1.0", + "@metamask/eth-qr-keyring": "npm:@metamask-previews/eth-qr-keyring@0.0.0-a9b0f29", "@metamask/eth-query": "^4.0.0", "@metamask/eth-sig-util": "^8.0.0", "@metamask/eth-snap-keyring": "^13.0.0", @@ -248,7 +249,6 @@ "@metamask/token-search-discovery-controller": "^3.1.0", "@metamask/transaction-controller": "^58.1.1", "@metamask/utils": "^11.4.2", - "@metamask/eth-qr-keyring": "../accounts/packages/keyring-eth-qr/", "@ngraveio/bc-ur": "^1.1.6", "@noble/hashes": "^1.7.1", "@notifee/react-native": "^9.0.0", @@ -294,6 +294,7 @@ "@xmldom/xmldom": "^0.8.10", "appium-adb": "^9.11.4", "asyncstorage-down": "4.2.0", + "async-mutex": "^0.5.0", "axios": "^1.8.2", "base-64": "1.0.0", "bignumber.js": "^9.0.1", diff --git a/yarn.lock b/yarn.lock index 55f9eaa77837..b8e0d4899368 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5142,17 +5142,18 @@ "@metamask/eth-sig-util" "^8.2.0" hdkey "^2.1.0" -"@metamask/eth-qr-keyring@../accounts/packages/keyring-eth-qr/": - version "0.0.0" +"@metamask/eth-qr-keyring@npm:@metamask-previews/eth-qr-keyring@0.0.0-a9b0f29": + version "0.0.0-a9b0f29" + resolved "https://registry.yarnpkg.com/@metamask-previews/eth-qr-keyring/-/eth-qr-keyring-0.0.0-a9b0f29.tgz#d42d886880db1474cb7f75137e10bbbab7de8b85" + integrity sha512-x/U7jCTJtFS8eqaXuwWitMhc2Qz05IZkIxVcSsvuM+a/4+tG+1BnqYSqJePb8Av+ncky+MFR9zzInUeJrBU2fQ== dependencies: "@ethereumjs/rlp" "^5.0.2" "@ethereumjs/tx" "^5.4.0" "@ethereumjs/util" "^9.1.0" "@keystonehq/bc-ur-registry-eth" "^0.19.1" "@metamask/eth-sig-util" "^8.2.0" - "@metamask/keyring-utils" "^3.0.0" + "@metamask/keyring-utils" "3.0.0" "@metamask/utils" "^11.1.0" - async-mutex "^0.5.0" hdkey "^2.1.0" uuid "^9.0.1" @@ -5389,7 +5390,7 @@ uuid "^9.0.1" webextension-polyfill "^0.12.0" -"@metamask/keyring-utils@^3.0.0": +"@metamask/keyring-utils@3.0.0", "@metamask/keyring-utils@^3.0.0": version "3.0.0" resolved "https://registry.yarnpkg.com/@metamask/keyring-utils/-/keyring-utils-3.0.0.tgz#37ad4b948e7306ef7d157ed074377073526ea143" integrity sha512-bo6Bgmz2uJ+DI5r9qyZjp435VCrc0A4nES/awTPnRWETqpbOBfMDVlLeQxQE88exCrE8efK2hTJ1by5fw8+N2g== From 02343463f3ad16fbdd680ea717ef24cfce2346b3 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 15 Jul 2025 12:00:00 +0200 Subject: [PATCH 03/31] move deferred promise to `QrKeyringScanner` --- .../UI/QRHardware/QRSigningDetails.tsx | 2 +- .../Views/ConnectQRHardware/index.tsx | 6 +- .../components/qr-info/qr-info.tsx | 4 +- .../qr-hardware-context.test.tsx | 12 ++- .../qr-hardware-context.tsx | 8 +- app/core/Engine/Engine.ts | 73 +++++-------------- app/core/QrKeyring/QrKeyringScanner.ts | 60 +++++++++++++++ 7 files changed, 100 insertions(+), 65 deletions(-) create mode 100644 app/core/QrKeyring/QrKeyringScanner.ts diff --git a/app/components/UI/QRHardware/QRSigningDetails.tsx b/app/components/UI/QRHardware/QRSigningDetails.tsx index 783b9591b6e2..bc22e58cfe18 100644 --- a/app/components/UI/QRHardware/QRSigningDetails.tsx +++ b/app/components/UI/QRHardware/QRSigningDetails.tsx @@ -229,7 +229,7 @@ const QRSigningDetails = ({ if (buffer) { const requestId = uuidStringify(buffer); if (pendingScanRequest?.request?.requestId === requestId) { - Engine.resolveQrKeyringScanRequest({ + Engine.getQrKeyringScanner().resolvePendingScan({ type: ur.type, cbor: ur.cbor.toString('hex'), }); diff --git a/app/components/Views/ConnectQRHardware/index.tsx b/app/components/Views/ConnectQRHardware/index.tsx index dfb7e2335caf..c9de4afa8c78 100644 --- a/app/components/Views/ConnectQRHardware/index.tsx +++ b/app/components/Views/ConnectQRHardware/index.tsx @@ -139,7 +139,7 @@ const ConnectQRHardware = ({ navigation }: IConnectQRHardwareProps) => { }) .build(), ); - Engine.resolveQrKeyringScanRequest({ + Engine.getQrKeyringScanner().resolvePendingScan({ type: ur.type, cbor: ur.cbor.toString('hex'), }); @@ -150,11 +150,11 @@ const ConnectQRHardware = ({ navigation }: IConnectQRHardwareProps) => { const onScanError = useCallback(async (error: string) => { setErrorMsg(error); - Engine.rejectQrKeyringScanRequest(new Error(error)); + Engine.getQrKeyringScanner().rejectPendingScan(new Error(error)); }, []); const cancelScan = useCallback(() => { - Engine.rejectQrKeyringScanRequest(new Error('Scan cancelled')); + Engine.getQrKeyringScanner().rejectPendingScan(new Error('Scan cancelled')); }, []); const nextPage = useCallback(async () => { diff --git a/app/components/Views/confirmations/components/qr-info/qr-info.tsx b/app/components/Views/confirmations/components/qr-info/qr-info.tsx index 93e8d1964194..b3302478532c 100644 --- a/app/components/Views/confirmations/components/qr-info/qr-info.tsx +++ b/app/components/Views/confirmations/components/qr-info/qr-info.tsx @@ -44,7 +44,7 @@ const QRInfo = () => { if (buffer) { const requestId = uuidStringify(buffer); if (pendingScanRequest?.request?.requestId === requestId) { - Engine.resolveQrKeyringScanRequest({ + Engine.getQrKeyringScanner().resolvePendingScan({ type: ur.type, cbor: ur.cbor.toString('hex'), }); @@ -120,7 +120,7 @@ const QRInfo = () => { setScannerVisible(false)} diff --git a/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.test.tsx b/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.test.tsx index f18f17f7d25b..ca96497157cf 100644 --- a/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.test.tsx +++ b/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.test.tsx @@ -2,7 +2,6 @@ import React from 'react'; import { userEvent } from '@testing-library/react-native'; import { ConfirmationFooterSelectorIDs } from '../../../../../../e2e/selectors/Confirmation/ConfirmationView.selectors'; -import Engine from '../../../../../core/Engine'; import renderWithProvider from '../../../../../util/test/renderWithProvider'; import { personalSignatureConfirmationState } from '../../../../../util/test/confirm-data-helpers'; import { Footer } from '../../components/footer'; @@ -17,6 +16,12 @@ import { } from './qr-hardware-context'; import { QrScanRequest, QrScanRequestType } from '@metamask/eth-qr-keyring'; +const mockQrScanner = { + requestScan: jest.fn(), + resolvePendingScan: jest.fn(), + rejectPendingScan: jest.fn(), +}; + jest.mock('../../../../../core/Engine', () => ({ context: { KeyringController: { @@ -29,6 +34,7 @@ jest.mock('../../../../../core/Engine', () => ({ unsubscribe: jest.fn(), }, rejectPendingApproval: jest.fn(), + getQrKeyringScanner: jest.fn(() => mockQrScanner), })); jest.mock('@react-navigation/native', () => ({ @@ -110,7 +116,7 @@ describe('QRHardwareContext', () => { }, ); await userEvent.press(getByText('Cancel')); - expect(Engine.rejectQrKeyringScanRequest).toHaveBeenCalledTimes(0); + expect(mockQrScanner.rejectPendingScan).toHaveBeenCalledTimes(0); }); it('invokes KeyringController.cancelQRSignRequest when request is cancelled', async () => { @@ -129,7 +135,7 @@ describe('QRHardwareContext', () => { }, ); await userEvent.press(getByText('Cancel')); - expect(Engine.rejectQrKeyringScanRequest).toHaveBeenCalledTimes(1); + expect(mockQrScanner.rejectPendingScan).toHaveBeenCalledTimes(1); }); it('passes correct value of QRState components', () => { diff --git a/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.tsx b/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.tsx index 5e6514770ee7..4452205b9957 100644 --- a/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.tsx +++ b/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.tsx @@ -53,7 +53,9 @@ export const QRHardwareContextProvider: React.FC<{ return; } e.preventDefault(); - Engine.rejectQrKeyringScanRequest(new Error('Request cancelled')); + Engine.getQrKeyringScanner().rejectPendingScan( + new Error('Request cancelled'), + ); navigation.dispatch(e.data.action); }, [isRequestCompleted, navigation], @@ -68,7 +70,9 @@ export const QRHardwareContextProvider: React.FC<{ if (!isQRSigningInProgress) { return; } - Engine.rejectQrKeyringScanRequest(new Error('Request cancelled')); + Engine.getQrKeyringScanner().rejectPendingScan( + new Error('Request cancelled'), + ); setRequestCompleted(true); setScannerVisible(false); }, [isQRSigningInProgress, setRequestCompleted, setScannerVisible]); diff --git a/app/core/Engine/Engine.ts b/app/core/Engine/Engine.ts index a00791d4b588..6d1902020244 100644 --- a/app/core/Engine/Engine.ts +++ b/app/core/Engine/Engine.ts @@ -115,12 +115,7 @@ import { unrestrictedMethods, } from '../Permissions/specifications.js'; import { backupVault } from '../BackupVault'; -import { - DeferredPromise, - Hex, - Json, - createDeferredPromise, -} from '@metamask/utils'; +import { Hex, Json } from '@metamask/utils'; import { providerErrors } from '@metamask/rpc-errors'; import { PPOM, ppomInit } from '../../lib/ppom/PPOMView'; @@ -242,6 +237,7 @@ import { WebSocketServiceInit } from './controllers/snaps/websocket-service-init import { seedlessOnboardingControllerInit } from './controllers/seedless-onboarding-controller'; import { scanCompleted, scanRequested } from '../redux/slices/qrKeyringScanner'; +import { QrKeyringScanner } from '../QrKeyring/QrKeyringScanner'; const NON_EMPTY = 'NON_EMPTY'; @@ -293,11 +289,17 @@ export class Engine { transactionController: TransactionController; multichainRouter: MultichainRouter; - #pendingQrKeyringScan: { - request: DeferredPromise | null; - } = { - request: null, - }; + readonly qrKeyringScanner = new QrKeyringScanner({ + onScanRequested: (request) => { + store.dispatch(scanRequested(request)); + }, + onScanResolved: () => { + store.dispatch(scanCompleted()); + }, + onScanRejected: () => { + store.dispatch(scanCompleted()); + }, + }); /** * Creates a CoreController instance @@ -570,7 +572,7 @@ export class Engine { const additionalKeyrings = []; const qrKeyringBridge = new QrKeyringScannerBridge({ - requestScan: (request) => this.requestQrKeyringScan(request), + requestScan: (request) => this.qrKeyringScanner.requestScan(request), }); const qrKeyringBuilder = () => { const keyring = new QrKeyring({ bridge: qrKeyringBridge }); @@ -2272,38 +2274,6 @@ export class Engine { } } - async requestQrKeyringScan(request: QrScanRequest): Promise { - if (this.#pendingQrKeyringScan.request) { - throw new Error( - 'A QR keyring scan request is already pending. Please wait for it to complete before requesting a new scan.', - ); - } - const deferredPromise = createDeferredPromise({ - suppressUnhandledRejection: true, - }); - this.#pendingQrKeyringScan.request = deferredPromise; - store.dispatch(scanRequested(request)); - return deferredPromise.promise; - } - - resolveQrKeyringScanRequest(response: SerializedUR) { - if (!this.#pendingQrKeyringScan.request) { - throw new Error('No pending QR keyring scan request to resolve.'); - } - this.#pendingQrKeyringScan.request.resolve(response); - this.#pendingQrKeyringScan.request = null; - store.dispatch(scanCompleted()); - } - - rejectQrKeyringScanRequest(error: Error) { - if (!this.#pendingQrKeyringScan.request) { - throw new Error('No pending QR keyring scan request to reject.'); - } - this.#pendingQrKeyringScan.request.reject(error); - this.#pendingQrKeyringScan.request = null; - store.dispatch(scanCompleted()); - } - // This should be used instead of directly calling PreferencesController.setSelectedAddress or AccountsController.setSelectedAccount setSelectedAccount(address: string) { const { AccountsController, PreferencesController } = this.context; @@ -2520,16 +2490,6 @@ export default { } = {}, ) => instance?.rejectPendingApproval(id, reason, opts), - resolveQrKeyringScanRequest: (response: SerializedUR) => { - assertEngineExists(instance); - instance.resolveQrKeyringScanRequest(response); - }, - - rejectQrKeyringScanRequest: (error: Error) => { - assertEngineExists(instance); - instance.rejectQrKeyringScanRequest(error); - }, - setSelectedAddress: (address: string) => { assertEngineExists(instance); instance.setSelectedAccount(address); @@ -2540,6 +2500,11 @@ export default { instance.setAccountLabel(address, label); }, + getQrKeyringScanner: () => { + assertEngineExists(instance); + return instance.qrKeyringScanner; + }, + ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) getSnapKeyring: () => { assertEngineExists(instance); diff --git a/app/core/QrKeyring/QrKeyringScanner.ts b/app/core/QrKeyring/QrKeyringScanner.ts new file mode 100644 index 000000000000..92700de7b5f9 --- /dev/null +++ b/app/core/QrKeyring/QrKeyringScanner.ts @@ -0,0 +1,60 @@ +import { QrScanRequest, SerializedUR } from '@metamask/eth-qr-keyring'; +import { DeferredPromise, createDeferredPromise } from '@metamask/utils'; +import { Mutex } from 'async-mutex'; + +export interface QrKeyringScannerOptions { + onScanRequested?: (request: QrScanRequest) => void; + onScanResolved?: (result: SerializedUR) => void; + onScanRejected?: (error: Error) => void; +} + +export class QrKeyringScanner { + readonly #lock = new Mutex(); + + readonly #onScanRequested?: (request: QrScanRequest) => void; + + readonly #onScanResolved?: (result: SerializedUR) => void; + + readonly #onScanRejected?: (error: Error) => void; + + #pendingScan?: DeferredPromise | null; + + constructor({ + onScanRequested, + onScanResolved, + onScanRejected, + }: QrKeyringScannerOptions = {}) { + this.#onScanRequested = onScanRequested; + this.#onScanResolved = onScanResolved; + this.#onScanRejected = onScanRejected; + } + + requestScan(request: QrScanRequest): Promise { + return this.#lock.runExclusive(() => { + if (this.#pendingScan) { + throw new Error('A scan is already pending.'); + } + this.#pendingScan = createDeferredPromise(); + this.#onScanRequested?.(request); + return this.#pendingScan.promise; + }); + } + + resolvePendingScan(result: SerializedUR): void { + if (!this.#pendingScan) { + throw new Error('No pending scan to resolve.'); + } + this.#pendingScan.resolve(result); + this.#pendingScan = null; + this.#onScanResolved?.(result); + } + + rejectPendingScan(error: Error): void { + if (!this.#pendingScan) { + throw new Error('No pending scan to reject.'); + } + this.#pendingScan.reject(error); + this.#pendingScan = null; + this.#onScanRejected?.(error); + } +} From 99302b59511ec541f9be4363f644bd3e5afbf0ad Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 15 Jul 2025 12:06:21 +0200 Subject: [PATCH 04/31] remove unused imports --- app/core/Engine/Engine.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/core/Engine/Engine.ts b/app/core/Engine/Engine.ts index 6d1902020244..0ee78587d41f 100644 --- a/app/core/Engine/Engine.ts +++ b/app/core/Engine/Engine.ts @@ -60,12 +60,7 @@ import { } from '@metamask/snaps-rpc-methods'; import type { EnumToUnion, DialogType } from '@metamask/snaps-sdk'; ///: END:ONLY_INCLUDE_IF -import { - QrKeyring, - QrKeyringScannerBridge, - QrScanRequest, - SerializedUR, -} from '@metamask/eth-qr-keyring'; +import { QrKeyring, QrKeyringScannerBridge } from '@metamask/eth-qr-keyring'; import { LoggingController } from '@metamask/logging-controller'; import { TokenSearchDiscoveryControllerMessenger } from '@metamask/token-search-discovery-controller'; import { From 31505fea603b2841691028621420ba93cae1b905 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 15 Jul 2025 12:14:59 +0200 Subject: [PATCH 05/31] remove unused redux action --- app/actions/modals/index.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/app/actions/modals/index.js b/app/actions/modals/index.js index d9d997be5a6f..8d382f2b70e3 100644 --- a/app/actions/modals/index.js +++ b/app/actions/modals/index.js @@ -31,10 +31,3 @@ export function toggleSignModal(show) { show, }; } - -export function toggleQrKeyringScanModal(show) { - return { - type: 'TOGGLE_QR_KEYRING_SCAN_MODAL', - show, - }; -} From 52cb8ab8ef30dc3e09bdcab6cf92c04fa1365257 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 15 Jul 2025 13:15:18 +0200 Subject: [PATCH 06/31] use `QrKeyringDeferredPromiseBridge` --- app/core/Engine/Engine.ts | 14 +++--- app/core/QrKeyring/QrKeyringScanner.ts | 60 -------------------------- package.json | 2 +- yarn.lock | 9 ++-- 4 files changed, 13 insertions(+), 72 deletions(-) delete mode 100644 app/core/QrKeyring/QrKeyringScanner.ts diff --git a/app/core/Engine/Engine.ts b/app/core/Engine/Engine.ts index 0ee78587d41f..174df0db5799 100644 --- a/app/core/Engine/Engine.ts +++ b/app/core/Engine/Engine.ts @@ -60,7 +60,11 @@ import { } from '@metamask/snaps-rpc-methods'; import type { EnumToUnion, DialogType } from '@metamask/snaps-sdk'; ///: END:ONLY_INCLUDE_IF -import { QrKeyring, QrKeyringScannerBridge } from '@metamask/eth-qr-keyring'; +import { + QrKeyring, + QrKeyringScannerBridge, + QrKeyringDeferredPromiseBridge, +} from '@metamask/eth-qr-keyring'; import { LoggingController } from '@metamask/logging-controller'; import { TokenSearchDiscoveryControllerMessenger } from '@metamask/token-search-discovery-controller'; import { @@ -232,7 +236,6 @@ import { WebSocketServiceInit } from './controllers/snaps/websocket-service-init import { seedlessOnboardingControllerInit } from './controllers/seedless-onboarding-controller'; import { scanCompleted, scanRequested } from '../redux/slices/qrKeyringScanner'; -import { QrKeyringScanner } from '../QrKeyring/QrKeyringScanner'; const NON_EMPTY = 'NON_EMPTY'; @@ -284,7 +287,7 @@ export class Engine { transactionController: TransactionController; multichainRouter: MultichainRouter; - readonly qrKeyringScanner = new QrKeyringScanner({ + readonly qrKeyringScanner = new QrKeyringDeferredPromiseBridge({ onScanRequested: (request) => { store.dispatch(scanRequested(request)); }, @@ -566,11 +569,8 @@ export class Engine { const additionalKeyrings = []; - const qrKeyringBridge = new QrKeyringScannerBridge({ - requestScan: (request) => this.qrKeyringScanner.requestScan(request), - }); const qrKeyringBuilder = () => { - const keyring = new QrKeyring({ bridge: qrKeyringBridge }); + const keyring = new QrKeyring({ bridge: this.qrKeyringScanner }); // to fix the bug in #9560, forgetDevice will reset all keyring properties to default. keyring.forgetDevice(); return keyring; diff --git a/app/core/QrKeyring/QrKeyringScanner.ts b/app/core/QrKeyring/QrKeyringScanner.ts deleted file mode 100644 index 92700de7b5f9..000000000000 --- a/app/core/QrKeyring/QrKeyringScanner.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { QrScanRequest, SerializedUR } from '@metamask/eth-qr-keyring'; -import { DeferredPromise, createDeferredPromise } from '@metamask/utils'; -import { Mutex } from 'async-mutex'; - -export interface QrKeyringScannerOptions { - onScanRequested?: (request: QrScanRequest) => void; - onScanResolved?: (result: SerializedUR) => void; - onScanRejected?: (error: Error) => void; -} - -export class QrKeyringScanner { - readonly #lock = new Mutex(); - - readonly #onScanRequested?: (request: QrScanRequest) => void; - - readonly #onScanResolved?: (result: SerializedUR) => void; - - readonly #onScanRejected?: (error: Error) => void; - - #pendingScan?: DeferredPromise | null; - - constructor({ - onScanRequested, - onScanResolved, - onScanRejected, - }: QrKeyringScannerOptions = {}) { - this.#onScanRequested = onScanRequested; - this.#onScanResolved = onScanResolved; - this.#onScanRejected = onScanRejected; - } - - requestScan(request: QrScanRequest): Promise { - return this.#lock.runExclusive(() => { - if (this.#pendingScan) { - throw new Error('A scan is already pending.'); - } - this.#pendingScan = createDeferredPromise(); - this.#onScanRequested?.(request); - return this.#pendingScan.promise; - }); - } - - resolvePendingScan(result: SerializedUR): void { - if (!this.#pendingScan) { - throw new Error('No pending scan to resolve.'); - } - this.#pendingScan.resolve(result); - this.#pendingScan = null; - this.#onScanResolved?.(result); - } - - rejectPendingScan(error: Error): void { - if (!this.#pendingScan) { - throw new Error('No pending scan to reject.'); - } - this.#pendingScan.reject(error); - this.#pendingScan = null; - this.#onScanRejected?.(error); - } -} diff --git a/package.json b/package.json index f29e4a573c07..d16f1b9b8e45 100644 --- a/package.json +++ b/package.json @@ -190,7 +190,7 @@ "@metamask/eth-json-rpc-filters": "^9.0.0", "@metamask/eth-json-rpc-middleware": "^17.0.1", "@metamask/eth-ledger-bridge-keyring": "11.1.0", - "@metamask/eth-qr-keyring": "npm:@metamask-previews/eth-qr-keyring@0.0.0-a9b0f29", + "@metamask/eth-qr-keyring": "npm:@metamask-previews/eth-qr-keyring@0.0.0-d2e6996", "@metamask/eth-query": "^4.0.0", "@metamask/eth-sig-util": "^8.0.0", "@metamask/eth-snap-keyring": "^13.0.0", diff --git a/yarn.lock b/yarn.lock index b8e0d4899368..7190ae38aaff 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5142,10 +5142,10 @@ "@metamask/eth-sig-util" "^8.2.0" hdkey "^2.1.0" -"@metamask/eth-qr-keyring@npm:@metamask-previews/eth-qr-keyring@0.0.0-a9b0f29": - version "0.0.0-a9b0f29" - resolved "https://registry.yarnpkg.com/@metamask-previews/eth-qr-keyring/-/eth-qr-keyring-0.0.0-a9b0f29.tgz#d42d886880db1474cb7f75137e10bbbab7de8b85" - integrity sha512-x/U7jCTJtFS8eqaXuwWitMhc2Qz05IZkIxVcSsvuM+a/4+tG+1BnqYSqJePb8Av+ncky+MFR9zzInUeJrBU2fQ== +"@metamask/eth-qr-keyring@npm:@metamask-previews/eth-qr-keyring@0.0.0-d2e6996": + version "0.0.0-d2e6996" + resolved "https://registry.yarnpkg.com/@metamask-previews/eth-qr-keyring/-/eth-qr-keyring-0.0.0-d2e6996.tgz#0b51fa1af370cee94a7db1d99e7fea5d411e3dab" + integrity sha512-AkBh5uyM/IPxRS/2ZaUvAwtYi0t3IvvxVtACV4sP6LGdbvfgSCOJXty8ChSJp4EXRrsPSKXxaTyTUUU+FJCCpg== dependencies: "@ethereumjs/rlp" "^5.0.2" "@ethereumjs/tx" "^5.4.0" @@ -5154,6 +5154,7 @@ "@metamask/eth-sig-util" "^8.2.0" "@metamask/keyring-utils" "3.0.0" "@metamask/utils" "^11.1.0" + async-mutex "^0.5.0" hdkey "^2.1.0" uuid "^9.0.1" From d6971ac80e12c3a75ac7e7bf563dc6547a1f6d04 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 15 Jul 2025 14:30:33 +0200 Subject: [PATCH 07/31] remove test resolution --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index d16f1b9b8e45..306e1586f6e8 100644 --- a/package.json +++ b/package.json @@ -158,8 +158,7 @@ "multihashes/**/base-x": "3.0.11", "@keystonehq/ur-decoder/**/base-x": "3.0.11", "@ethereumjs/tx": "^5.4.0", - "@metamask/controller-utils": "npm:@metamask/controller-utils@^11.11.0", - "@metamask/keyring-controller": "npm:@metamask-previews/keyring-controller@22.0.2-preview-d7c00b1" + "@metamask/controller-utils": "npm:@metamask/controller-utils@^11.11.0" }, "dependencies": { "@config-plugins/detox": "^9.0.0", From 8242f97873c70723761775c1c348ba436d375d49 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 15 Jul 2025 14:54:42 +0200 Subject: [PATCH 08/31] wip fix tests --- .../Views/ConnectQRHardware/index.test.tsx | 51 +++++++------------ .../Views/ConnectQRHardware/index.tsx | 2 +- app/core/QrKeyring/QrKeyring.ts | 2 +- 3 files changed, 21 insertions(+), 34 deletions(-) diff --git a/app/components/Views/ConnectQRHardware/index.test.tsx b/app/components/Views/ConnectQRHardware/index.test.tsx index e0c7b6a836b9..68a2205e146c 100644 --- a/app/components/Views/ConnectQRHardware/index.test.tsx +++ b/app/components/Views/ConnectQRHardware/index.test.tsx @@ -6,11 +6,11 @@ import { fireEvent } from '@testing-library/react-native'; import { QR_CONTINUE_BUTTON } from '../../../../wdio/screen-objects/testIDs/Components/ConnectQRHardware.testIds'; import { backgroundState } from '../../../util/test/initial-root-state'; import { act } from '@testing-library/react-hooks'; -import PAGINATION_OPERATIONS from '../../../constants/pagination'; import { ACCOUNT_SELECTOR_NEXT_BUTTON, ACCOUNT_SELECTOR_PREVIOUS_BUTTON, } from '../../../../wdio/screen-objects/testIDs/Components/AccountSelector.testIds'; +import { QrKeyringBridge } from '@metamask/eth-qr-keyring'; const mockedNavigate = jest.fn(); @@ -80,6 +80,16 @@ const mockPage1Accounts = [ }, ]; +const mockQrKeyring = { + getFirstPage: jest.fn(), + getNextPage: jest.fn(), + getPreviousPage: jest.fn(), +}; + +const mockQrKeyringBridge: QrKeyringBridge = { + requestScan: jest.fn(), +}; + jest.mock('@react-navigation/native', () => { const actualNav = jest.requireActual('@react-navigation/native'); return { @@ -98,14 +108,9 @@ jest.mock('../../../core/Engine', () => ({ keyrings: [], }, getAccounts: jest.fn(), - getOrAddQRKeyring: jest.fn(), withKeyring: (_selector: unknown, operation: (args: unknown) => void) => operation({ - keyring: { - cancelSync: jest.fn(), - submitCryptoAccount: jest.fn(), - submitCryptoHDKey: jest.fn(), - }, + keyring: mockQrKeyring, metadata: { id: '1234' }, }), connectQRHardware: jest.fn(), @@ -118,6 +123,7 @@ jest.mock('../../../core/Engine', () => ({ subscribe: jest.fn(), unsubscribe: jest.fn(), }, + qrKeyringScanner: mockQrKeyringBridge, })); const MockEngine = jest.mocked(Engine); @@ -131,19 +137,9 @@ const mockInitialState = { describe('ConnectQRHardware', () => { const mockKeyringController = MockEngine.context.KeyringController; - mockKeyringController.connectQRHardware.mockImplementation((page) => { - switch (page) { - case PAGINATION_OPERATIONS.GET_NEXT_PAGE: - return Promise.resolve(mockPage1Accounts); - - case PAGINATION_OPERATIONS.GET_PREVIOUS_PAGE: - return Promise.resolve(mockPage0Accounts); - - default: - // return account lists in first page. - return Promise.resolve(mockPage0Accounts); - } - }); + mockQrKeyring.getFirstPage.mockResolvedValue(mockPage0Accounts); + mockQrKeyring.getNextPage.mockResolvedValue(mockPage1Accounts); + mockQrKeyring.getPreviousPage.mockResolvedValue(mockPage0Accounts); const mockAccountTrackerController = MockEngine.context.AccountTrackerController; @@ -192,10 +188,7 @@ describe('ConnectQRHardware', () => { fireEvent.press(button); }); - expect(mockKeyringController.connectQRHardware).toHaveBeenCalledTimes(1); - expect(mockKeyringController.connectQRHardware).toHaveBeenCalledWith( - PAGINATION_OPERATIONS.GET_FIRST_PAGE, - ); + expect(mockQrKeyring.getFirstPage).toHaveBeenCalledTimes(1); mockPage0Accounts.forEach((account) => { expect(getByText(account.shortenedAddress)).toBeDefined(); @@ -223,10 +216,7 @@ describe('ConnectQRHardware', () => { fireEvent.press(nextButton); }); - expect(mockKeyringController.connectQRHardware).toHaveBeenCalledTimes(2); - expect(mockKeyringController.connectQRHardware).toHaveBeenCalledWith( - PAGINATION_OPERATIONS.GET_NEXT_PAGE, - ); + expect(mockQrKeyring.getNextPage).toHaveBeenCalledTimes(1); mockPage1Accounts.forEach((account) => { expect(getByText(account.shortenedAddress)).toBeDefined(); @@ -260,10 +250,7 @@ describe('ConnectQRHardware', () => { fireEvent.press(prevButton); }); - expect(mockKeyringController.connectQRHardware).toHaveBeenCalledTimes(3); - expect(mockKeyringController.connectQRHardware).toHaveBeenCalledWith( - PAGINATION_OPERATIONS.GET_PREVIOUS_PAGE, - ); + expect(mockQrKeyring.getPreviousPage).toHaveBeenCalledTimes(1); mockPage0Accounts.forEach((account) => { expect(getByText(account.shortenedAddress)).toBeDefined(); diff --git a/app/components/Views/ConnectQRHardware/index.tsx b/app/components/Views/ConnectQRHardware/index.tsx index c9de4afa8c78..2004379bb78e 100644 --- a/app/components/Views/ConnectQRHardware/index.tsx +++ b/app/components/Views/ConnectQRHardware/index.tsx @@ -29,7 +29,7 @@ import ExtendedKeyringTypes, { } from '../../../constants/keyringTypes'; import { ThemeColors } from '@metamask/design-tokens'; import { QrScanRequestType } from '@metamask/eth-qr-keyring'; -import { withQrKeyring } from 'app/core/QrKeyring/QrKeyring'; +import { withQrKeyring } from '../../../core/QrKeyring/QrKeyring'; import { getChecksumAddress } from '@metamask/utils'; interface IConnectQRHardwareProps { diff --git a/app/core/QrKeyring/QrKeyring.ts b/app/core/QrKeyring/QrKeyring.ts index 529c5e170ba2..8fc68e3c23bd 100644 --- a/app/core/QrKeyring/QrKeyring.ts +++ b/app/core/QrKeyring/QrKeyring.ts @@ -1,7 +1,7 @@ import { QrKeyring } from '@metamask/eth-qr-keyring'; import { KeyringMetadata } from '@metamask/keyring-controller'; import Engine from '../Engine'; -import ExtendedKeyringTypes from 'app/constants/keyringTypes'; +import ExtendedKeyringTypes from '../../constants/keyringTypes'; /** * Perform an operation with the QR keyring. From 495c2cc663ca420fab61b30978d53c6c6c6ff8ee Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 15 Jul 2025 15:54:38 +0200 Subject: [PATCH 09/31] remove `getOrAddQRKeyring` usage --- app/components/Views/ActivityView/index.test.tsx | 5 ----- app/components/Views/Asset/index.test.js | 1 - app/components/Views/ConnectQRHardware/index.test.tsx | 1 - app/components/Views/TransactionsView/index.test.tsx | 1 - .../components/confirm/confirm-component.test.tsx | 3 +-- .../components/info-root/info-root.test.tsx | 1 - .../info/personal-sign/personal-sign.test.tsx | 1 - .../switch-account-type/switch-account-type.test.tsx | 1 - .../info/typed-sign-v1/typed-sign-v1.test.tsx | 3 +-- .../info/typed-sign-v3v4/typed-sign-v3v4.test.tsx | 10 ++++++---- .../switch-account-type-info-row.test.tsx | 1 - .../qr-hardware-context/qr-hardware-context.test.tsx | 1 - .../hooks/useConfirmationRedesignEnabled.test.ts | 1 - .../Views/confirmations/legacy/Approval/index.test.tsx | 1 - .../Views/confirmations/legacy/Approve/index.test.tsx | 3 --- .../legacy/ApproveView/Approve/index.test.tsx | 3 --- .../components/ApproveTransactionReview/index.test.jsx | 3 --- .../components/SignatureRequest/Root/Root.test.tsx | 1 - .../legacy/components/TypedSign/index.test.tsx | 5 +++-- 19 files changed, 11 insertions(+), 35 deletions(-) diff --git a/app/components/Views/ActivityView/index.test.tsx b/app/components/Views/ActivityView/index.test.tsx index 7bd8c7422566..d1fcda8aa96a 100644 --- a/app/components/Views/ActivityView/index.test.tsx +++ b/app/components/Views/ActivityView/index.test.tsx @@ -7,11 +7,6 @@ import { createStackNavigator } from '@react-navigation/stack'; const Stack = createStackNavigator(); jest.mock('../../../core/Engine', () => ({ - context: { - KeyringController: { - getOrAddQRKeyring: jest.fn(), - }, - }, controllerMessenger: { subscribe: jest.fn(), unsubscribe: jest.fn(), diff --git a/app/components/Views/Asset/index.test.js b/app/components/Views/Asset/index.test.js index cf0f9767d358..3d68e03a978b 100644 --- a/app/components/Views/Asset/index.test.js +++ b/app/components/Views/Asset/index.test.js @@ -235,7 +235,6 @@ jest.mock('../../../core/Engine', () => { return { context: { KeyringController: { - getOrAddQRKeyring: async () => ({ subscribe: () => ({}) }), state: { keyrings: [ { diff --git a/app/components/Views/ConnectQRHardware/index.test.tsx b/app/components/Views/ConnectQRHardware/index.test.tsx index 68a2205e146c..e01103325f28 100644 --- a/app/components/Views/ConnectQRHardware/index.test.tsx +++ b/app/components/Views/ConnectQRHardware/index.test.tsx @@ -113,7 +113,6 @@ jest.mock('../../../core/Engine', () => ({ keyring: mockQrKeyring, metadata: { id: '1234' }, }), - connectQRHardware: jest.fn(), }, AccountTrackerController: { syncBalanceWithAddresses: jest.fn(), diff --git a/app/components/Views/TransactionsView/index.test.tsx b/app/components/Views/TransactionsView/index.test.tsx index 5742199ce137..9ee5b20128a6 100644 --- a/app/components/Views/TransactionsView/index.test.tsx +++ b/app/components/Views/TransactionsView/index.test.tsx @@ -138,7 +138,6 @@ jest.mock('../../UI/Transactions', () => () => null); jest.mock('../../../core/Engine', () => ({ context: { KeyringController: { - getOrAddQRKeyring: jest.fn(), cancelQRSignRequest: jest.fn().mockResolvedValue(undefined), state: { keyrings: [], diff --git a/app/components/Views/confirmations/components/confirm/confirm-component.test.tsx b/app/components/Views/confirmations/components/confirm/confirm-component.test.tsx index 906794f18cea..3f6c1b2729f6 100644 --- a/app/components/Views/confirmations/components/confirm/confirm-component.test.tsx +++ b/app/components/Views/confirmations/components/confirm/confirm-component.test.tsx @@ -84,10 +84,9 @@ jest.mock('../../../../../core/Engine', () => ({ id: '01JNG7170V9X27V5NFDTY04PJ4', name: '', }, - } + }, ], }, - getOrAddQRKeyring: jest.fn(), }, NetworkController: { getNetworkConfigurationByNetworkClientId: jest.fn(), diff --git a/app/components/Views/confirmations/components/info-root/info-root.test.tsx b/app/components/Views/confirmations/components/info-root/info-root.test.tsx index 0ef9403f239b..3860d527b366 100644 --- a/app/components/Views/confirmations/components/info-root/info-root.test.tsx +++ b/app/components/Views/confirmations/components/info-root/info-root.test.tsx @@ -47,7 +47,6 @@ jest.mock('../../../../../core/Engine', () => ({ state: { keyrings: [], }, - getOrAddQRKeyring: jest.fn(), }, GasFeeController: { startPolling: jest.fn(), diff --git a/app/components/Views/confirmations/components/info/personal-sign/personal-sign.test.tsx b/app/components/Views/confirmations/components/info/personal-sign/personal-sign.test.tsx index 78e6cfeb4648..000a8463724e 100644 --- a/app/components/Views/confirmations/components/info/personal-sign/personal-sign.test.tsx +++ b/app/components/Views/confirmations/components/info/personal-sign/personal-sign.test.tsx @@ -15,7 +15,6 @@ jest.mock('../../../../../../core/Engine', () => ({ state: { keyrings: [], }, - getOrAddQRKeyring: jest.fn(), }, AccountsController: { state: { diff --git a/app/components/Views/confirmations/components/info/switch-account-type/switch-account-type.test.tsx b/app/components/Views/confirmations/components/info/switch-account-type/switch-account-type.test.tsx index dcb741f1612f..2952cb3f9ac4 100644 --- a/app/components/Views/confirmations/components/info/switch-account-type/switch-account-type.test.tsx +++ b/app/components/Views/confirmations/components/info/switch-account-type/switch-account-type.test.tsx @@ -19,7 +19,6 @@ jest.mock('../../../../../../core/Engine', () => ({ state: { keyrings: [], }, - getOrAddQRKeyring: jest.fn(), }, GasFeeController: { startPolling: jest.fn(), diff --git a/app/components/Views/confirmations/components/info/typed-sign-v1/typed-sign-v1.test.tsx b/app/components/Views/confirmations/components/info/typed-sign-v1/typed-sign-v1.test.tsx index c266ae9585b8..0df77395616c 100644 --- a/app/components/Views/confirmations/components/info/typed-sign-v1/typed-sign-v1.test.tsx +++ b/app/components/Views/confirmations/components/info/typed-sign-v1/typed-sign-v1.test.tsx @@ -11,7 +11,6 @@ jest.mock('../../../../../../core/Engine', () => ({ state: { keyrings: [], }, - getOrAddQRKeyring: jest.fn(), }, AccountsController: { state: { @@ -23,7 +22,7 @@ jest.mock('../../../../../../core/Engine', () => ({ metadata: { name: 'Account 1', keyring: { - type: 'HD Key Tree' + type: 'HD Key Tree', }, }, }, diff --git a/app/components/Views/confirmations/components/info/typed-sign-v3v4/typed-sign-v3v4.test.tsx b/app/components/Views/confirmations/components/info/typed-sign-v3v4/typed-sign-v3v4.test.tsx index 2d90a9b5bc2d..cf0c530ffb31 100644 --- a/app/components/Views/confirmations/components/info/typed-sign-v3v4/typed-sign-v3v4.test.tsx +++ b/app/components/Views/confirmations/components/info/typed-sign-v3v4/typed-sign-v3v4.test.tsx @@ -16,7 +16,6 @@ jest.mock('../../../../../../core/Engine', () => ({ state: { keyrings: [], }, - getOrAddQRKeyring: jest.fn(), }, NetworkController: { findNetworkClientIdByChainId: () => 123, @@ -45,9 +44,12 @@ jest.mock('../../../../../../core/Engine', () => ({ }, })); -jest.mock('../../../hooks/signatures/useTokenDecimalsInTypedSignRequest', () => ({ - useTokenDecimalsInTypedSignRequest: () => 2, -})); +jest.mock( + '../../../hooks/signatures/useTokenDecimalsInTypedSignRequest', + () => ({ + useTokenDecimalsInTypedSignRequest: () => 2, + }), +); describe('TypedSignV3V4', () => { it('contains required text', () => { diff --git a/app/components/Views/confirmations/components/rows/switch-account-type-info-row/switch-account-type-info-row.test.tsx b/app/components/Views/confirmations/components/rows/switch-account-type-info-row/switch-account-type-info-row.test.tsx index f12a18431065..caf6a9082581 100644 --- a/app/components/Views/confirmations/components/rows/switch-account-type-info-row/switch-account-type-info-row.test.tsx +++ b/app/components/Views/confirmations/components/rows/switch-account-type-info-row/switch-account-type-info-row.test.tsx @@ -20,7 +20,6 @@ jest.mock('../../../../../../core/Engine', () => ({ state: { keyrings: [], }, - getOrAddQRKeyring: jest.fn(), }, GasFeeController: { startPolling: jest.fn(), diff --git a/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.test.tsx b/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.test.tsx index ca96497157cf..dbd8e55e6806 100644 --- a/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.test.tsx +++ b/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.test.tsx @@ -25,7 +25,6 @@ const mockQrScanner = { jest.mock('../../../../../core/Engine', () => ({ context: { KeyringController: { - getOrAddQRKeyring: jest.fn(), cancelQRSignRequest: jest.fn().mockResolvedValue(undefined), }, }, diff --git a/app/components/Views/confirmations/hooks/useConfirmationRedesignEnabled.test.ts b/app/components/Views/confirmations/hooks/useConfirmationRedesignEnabled.test.ts index 5af28adc7584..ff94b84133eb 100644 --- a/app/components/Views/confirmations/hooks/useConfirmationRedesignEnabled.test.ts +++ b/app/components/Views/confirmations/hooks/useConfirmationRedesignEnabled.test.ts @@ -39,7 +39,6 @@ jest.mock('../../../../core/Engine', () => ({ state: { keyrings: [], }, - getOrAddQRKeyring: jest.fn(), }, }, controllerMessenger: { diff --git a/app/components/Views/confirmations/legacy/Approval/index.test.tsx b/app/components/Views/confirmations/legacy/Approval/index.test.tsx index 64d49c092ed9..20cc8cc6b645 100644 --- a/app/components/Views/confirmations/legacy/Approval/index.test.tsx +++ b/app/components/Views/confirmations/legacy/Approval/index.test.tsx @@ -34,7 +34,6 @@ jest.mock('../../../../../core/Engine', () => ({ context: { KeyringController: { resetQRKeyringState: jest.fn(), - getOrAddQRKeyring: jest.fn(), }, GasFeeController: { getGasFeeEstimatesAndStartPolling: jest.fn().mockResolvedValue(null), diff --git a/app/components/Views/confirmations/legacy/Approve/index.test.tsx b/app/components/Views/confirmations/legacy/Approve/index.test.tsx index 61b5b3f193bd..35cc2924a231 100644 --- a/app/components/Views/confirmations/legacy/Approve/index.test.tsx +++ b/app/components/Views/confirmations/legacy/Approve/index.test.tsx @@ -55,9 +55,6 @@ jest.mock('../../../../../core/Engine', () => ({ AssetsContractController: { getERC20BalanceOf: jest.fn().mockResolvedValue(null), }, - KeyringController: { - getOrAddQRKeyring: jest.fn(), - }, TransactionController: { getNonceLock: jest .fn() diff --git a/app/components/Views/confirmations/legacy/ApproveView/Approve/index.test.tsx b/app/components/Views/confirmations/legacy/ApproveView/Approve/index.test.tsx index 244c32456212..29076122a20c 100644 --- a/app/components/Views/confirmations/legacy/ApproveView/Approve/index.test.tsx +++ b/app/components/Views/confirmations/legacy/ApproveView/Approve/index.test.tsx @@ -55,9 +55,6 @@ jest.mock('../../../../../../core/Engine', () => ({ AssetsContractController: { getERC20BalanceOf: jest.fn().mockResolvedValue(null), }, - KeyringController: { - getOrAddQRKeyring: jest.fn(), - }, TransactionController: { getNonceLock: jest .fn() diff --git a/app/components/Views/confirmations/legacy/components/ApproveTransactionReview/index.test.jsx b/app/components/Views/confirmations/legacy/components/ApproveTransactionReview/index.test.jsx index d2db42724b0e..03876f017291 100644 --- a/app/components/Views/confirmations/legacy/components/ApproveTransactionReview/index.test.jsx +++ b/app/components/Views/confirmations/legacy/components/ApproveTransactionReview/index.test.jsx @@ -26,9 +26,6 @@ jest.mock('../../../../../../core/Engine', () => { ); return { context: { - KeyringController: { - getOrAddQRKeyring: async () => ({ subscribe: () => ({}) }), - }, AssetsContractController: { getERC20BalanceOf: jest.fn().mockResolvedValue(0x0186a0), }, diff --git a/app/components/Views/confirmations/legacy/components/SignatureRequest/Root/Root.test.tsx b/app/components/Views/confirmations/legacy/components/SignatureRequest/Root/Root.test.tsx index fc0edb808acd..d549fd308dee 100644 --- a/app/components/Views/confirmations/legacy/components/SignatureRequest/Root/Root.test.tsx +++ b/app/components/Views/confirmations/legacy/components/SignatureRequest/Root/Root.test.tsx @@ -23,7 +23,6 @@ jest.mock('../../../../../../../core/Engine', () => ({ getQRKeyringState: jest.fn(() => Promise.resolve({ subscribe: jest.fn(), unsubscribe: jest.fn() }), ), - getOrAddQRKeyring: jest.fn(), state: { keyrings: [], }, diff --git a/app/components/Views/confirmations/legacy/components/TypedSign/index.test.tsx b/app/components/Views/confirmations/legacy/components/TypedSign/index.test.tsx index adbee5b069d8..6b6c6bd9a60a 100644 --- a/app/components/Views/confirmations/legacy/components/TypedSign/index.test.tsx +++ b/app/components/Views/confirmations/legacy/components/TypedSign/index.test.tsx @@ -31,7 +31,9 @@ const mockMetrics = { jest.mock('../../../../../../core/Engine', () => { const { MOCK_ACCOUNTS_CONTROLLER_STATE: mockAccountsControllerState } = - jest.requireActual('../../../../../../util/test/accountsControllerTestUtils'); + jest.requireActual( + '../../../../../../util/test/accountsControllerTestUtils', + ); return { acceptPendingApproval: jest.fn(), rejectPendingApproval: jest.fn(), @@ -41,7 +43,6 @@ jest.mock('../../../../../../core/Engine', () => { keyrings: [], }, getAccountKeyringType: jest.fn(() => Promise.resolve({ data: {} })), - getOrAddQRKeyring: jest.fn(), }, SignatureController: { hub: { From 454ec27936ee23759ac380416d4327f0cf459d21 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 15 Jul 2025 15:56:07 +0200 Subject: [PATCH 10/31] remove `resetQRKeyringState()` usage --- app/components/Nav/Main/RootRPCMethodsUI.js | 2 -- app/components/UI/Transactions/index.js | 3 +-- app/components/UI/Transactions/index.test.tsx | 6 ------ .../Views/confirmations/legacy/Approval/index.js | 1 - .../Views/confirmations/legacy/Approve/index.js | 1 - .../confirmations/legacy/ApproveView/Approve/index.js | 1 - app/components/Views/confirmations/legacy/Send/index.js | 1 - .../Views/confirmations/legacy/SendFlow/Confirm/index.js | 8 +------- 8 files changed, 2 insertions(+), 21 deletions(-) diff --git a/app/components/Nav/Main/RootRPCMethodsUI.js b/app/components/Nav/Main/RootRPCMethodsUI.js index 63a297d82d09..acc795c75aae 100644 --- a/app/components/Nav/Main/RootRPCMethodsUI.js +++ b/app/components/Nav/Main/RootRPCMethodsUI.js @@ -321,8 +321,6 @@ const RootRPCMethodsUI = (props) => { // Queue txMetaId to listen for confirmation event addTransactionMetaIdForListening(transactionMeta.id); - await KeyringController.resetQRKeyringState(); - const isLedgerAccount = isHardwareAccount( transactionMeta.txParams.from, [ExtendedKeyringTypes.ledger], diff --git a/app/components/UI/Transactions/index.js b/app/components/UI/Transactions/index.js index 62187b3ef23b..fbdc5095d644 100644 --- a/app/components/UI/Transactions/index.js +++ b/app/components/UI/Transactions/index.js @@ -563,8 +563,7 @@ class Transactions extends PureComponent { }; signQRTransaction = async (tx) => { - const { KeyringController, ApprovalController } = Engine.context; - await KeyringController.resetQRKeyringState(); + const { ApprovalController } = Engine.context; await ApprovalController.accept(tx.id, undefined, { waitForResult: true }); }; diff --git a/app/components/UI/Transactions/index.test.tsx b/app/components/UI/Transactions/index.test.tsx index 8c752f73a90d..603545ea53b6 100644 --- a/app/components/UI/Transactions/index.test.tsx +++ b/app/components/UI/Transactions/index.test.tsx @@ -57,9 +57,6 @@ jest.mock('../../../util/transaction-controller', () => ({ jest.mock('../../../core/Engine', () => ({ context: { - KeyringController: { - resetQRKeyringState: jest.fn(), - }, ApprovalController: { accept: jest.fn(), reject: jest.fn(), @@ -641,9 +638,6 @@ describe('Transactions', () => { }); it('should test Engine context methods', () => { - expect( - Engine.context.KeyringController.resetQRKeyringState, - ).toBeDefined(); expect(Engine.context.ApprovalController.accept).toBeDefined(); expect( Engine.context.TransactionController.stopTransaction, diff --git a/app/components/Views/confirmations/legacy/Approval/index.js b/app/components/Views/confirmations/legacy/Approval/index.js index d452ae749f05..f25703484196 100644 --- a/app/components/Views/confirmations/legacy/Approval/index.js +++ b/app/components/Views/confirmations/legacy/Approval/index.js @@ -565,7 +565,6 @@ class Approval extends PureComponent { }, (transactionMeta) => transactionMeta.id === transaction.id, ); - await KeyringController.resetQRKeyringState(); const fullTx = transactions.find(({ id }) => id === transaction.id); diff --git a/app/components/Views/confirmations/legacy/Approve/index.js b/app/components/Views/confirmations/legacy/Approve/index.js index b2904ee3a212..d9e6c972e36d 100644 --- a/app/components/Views/confirmations/legacy/Approve/index.js +++ b/app/components/Views/confirmations/legacy/Approve/index.js @@ -585,7 +585,6 @@ class Approve extends PureComponent { }, }; await updateTransaction(updatedTx); - await KeyringController.resetQRKeyringState(); // For Ledger Accounts we handover the signing to the confirmation flow if (isLedgerAccount) { diff --git a/app/components/Views/confirmations/legacy/ApproveView/Approve/index.js b/app/components/Views/confirmations/legacy/ApproveView/Approve/index.js index 9ccfa897de7c..6d29cf4827e7 100644 --- a/app/components/Views/confirmations/legacy/ApproveView/Approve/index.js +++ b/app/components/Views/confirmations/legacy/ApproveView/Approve/index.js @@ -600,7 +600,6 @@ class Approve extends PureComponent { }, }; await updateTransaction(updatedTx); - await KeyringController.resetQRKeyringState(); // For Ledger Accounts we handover the signing to the confirmation flow if (isLedgerAccount) { diff --git a/app/components/Views/confirmations/legacy/Send/index.js b/app/components/Views/confirmations/legacy/Send/index.js index 8a83319ea7ba..054b069050e5 100644 --- a/app/components/Views/confirmations/legacy/Send/index.js +++ b/app/components/Views/confirmations/legacy/Send/index.js @@ -573,7 +573,6 @@ class Send extends PureComponent { networkClientId: globalNetworkClientId, origin: TransactionTypes.MMM, }); - await KeyringController.resetQRKeyringState(); await ApprovalController.accept(transactionMeta.id, undefined, { waitForResult: true, }); diff --git a/app/components/Views/confirmations/legacy/SendFlow/Confirm/index.js b/app/components/Views/confirmations/legacy/SendFlow/Confirm/index.js index cd4744bec07e..1775a057555c 100644 --- a/app/components/Views/confirmations/legacy/SendFlow/Confirm/index.js +++ b/app/components/Views/confirmations/legacy/SendFlow/Confirm/index.js @@ -1054,8 +1054,6 @@ class Confirm extends PureComponent { return; } - await KeyringController.resetQRKeyringState(); - if (shouldUseSmartTransaction) { await ApprovalController.accept(transactionMeta.id, undefined, { waitForResult: false, @@ -1215,11 +1213,7 @@ class Confirm extends PureComponent { style={styles.hexDataClose} onPress={this.toggleHexDataModal} > - + From bac14c4483f67b824b1fee782a4b9234b793b85b Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 15 Jul 2025 15:56:26 +0200 Subject: [PATCH 11/31] fix `qr-info.test.tsx` --- .../components/qr-info/qr-info.test.tsx | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/app/components/Views/confirmations/components/qr-info/qr-info.test.tsx b/app/components/Views/confirmations/components/qr-info/qr-info.test.tsx index 6b5a0f46ac8b..00dbc7cedea2 100644 --- a/app/components/Views/confirmations/components/qr-info/qr-info.test.tsx +++ b/app/components/Views/confirmations/components/qr-info/qr-info.test.tsx @@ -8,22 +8,21 @@ import { typedSignV3ConfirmationState } from '../../../../../util/test/confirm-d // eslint-disable-next-line import/no-namespace import * as QRHardwareHook from '../../context/qr-hardware-context/qr-hardware-context'; import QRInfo from './qr-info'; -import Engine from '../../../../../core/Engine'; + +const mockQrKeyringBridge = { + requestScan: jest.fn(), + resolvePendingScan: jest.fn(), + rejectPendingScan: jest.fn(), +}; jest.mock('../../../../../core/Engine', () => ({ - context: { - KeyringController: { - submitQRSignature: jest.fn(), - }, - }, + qrKeyringScanner: mockQrKeyringBridge, })); jest.mock('uuid', () => ({ stringify: jest.fn().mockReturnValue('c95ecc76-d6e9-4a0a-afa3-31429bc80566'), })); -const MockEngine = jest.mocked(Engine); - const MockView = View; const MockText = Text; const MockButton = Button; @@ -66,8 +65,6 @@ const mockQRState = { }; describe('QRInfo', () => { - - const mockKeyringController = MockEngine.context.KeyringController; const mockSetRequestCompleted = jest.fn(); beforeEach(() => { @@ -149,7 +146,7 @@ describe('QRInfo', () => { state: typedSignV3ConfirmationState, }); fireEvent.press(getByText('onScanSuccess')); - expect(mockKeyringController.submitQRSignature).toHaveBeenCalledTimes(1); + expect(mockQrKeyringBridge.resolvePendingScan).toHaveBeenCalledTimes(1); expect(mockSetRequestCompleted).toHaveBeenCalledTimes(1); expect(mockSetScannerVisible).toHaveBeenCalledTimes(1); expect(mockSetScannerVisible).toHaveBeenCalledWith(false); From 71088597937d6d310051a8ef208a56f4d869ef63 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 15 Jul 2025 21:57:11 +0200 Subject: [PATCH 12/31] remove `QRState` --- .../TransactionApproval.test.tsx | 5 ++++- .../ApproveTransactionReview/index.js | 7 +++---- .../legacy/components/SignatureRequest/index.js | 12 ++++-------- .../components/TransactionReview/index.js | 17 ++++++++++++----- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/app/components/Approvals/TransactionApproval/TransactionApproval.test.tsx b/app/components/Approvals/TransactionApproval/TransactionApproval.test.tsx index b534d116a225..2034161327e5 100644 --- a/app/components/Approvals/TransactionApproval/TransactionApproval.test.tsx +++ b/app/components/Approvals/TransactionApproval/TransactionApproval.test.tsx @@ -85,7 +85,10 @@ describe('TransactionApproval', () => { } as any); const wrapper = shallow( - , + , ); expect(wrapper).toMatchSnapshot(); diff --git a/app/components/Views/confirmations/legacy/components/ApproveTransactionReview/index.js b/app/components/Views/confirmations/legacy/components/ApproveTransactionReview/index.js index ebf2fedb712d..260197492192 100644 --- a/app/components/Views/confirmations/legacy/components/ApproveTransactionReview/index.js +++ b/app/components/Views/confirmations/legacy/components/ApproveTransactionReview/index.js @@ -236,7 +236,7 @@ class ApproveTransactionReview extends PureComponent { */ nicknameExists: PropTypes.bool, isSigningQRObject: PropTypes.bool, - QRState: PropTypes.object, + pendingScanRequest: PropTypes.object, /** * The selected gas value (low, medium, high). Gas value can be null when the advanced option is modified. */ @@ -472,7 +472,6 @@ class ApproveTransactionReview extends PureComponent { }, }); - this.setState( { host, @@ -1310,7 +1309,7 @@ class ApproveTransactionReview extends PureComponent { const { activeTabUrl, transaction: { origin, from }, - QRState, + pendingScanRequest, } = this.props; const styles = this.getStyles(); return ( @@ -1324,7 +1323,7 @@ class ApproveTransactionReview extends PureComponent { }} /> - + ); }; @@ -375,13 +371,13 @@ class SignatureRequest extends PureComponent { } renderQRDetails() { - const { QRState, fromAddress } = this.props; + const { pendingScanRequest, fromAddress } = this.props; const styles = this.getStyles(); return ( )} @@ -685,7 +692,7 @@ class TransactionReview extends PureComponent { renderQRDetails() { const currentPageInformation = { url: this.getUrlFromBrowser() }; const { - QRState, + pendingScanRequest, transaction: { from }, onCancel, onConfirm, @@ -696,7 +703,7 @@ class TransactionReview extends PureComponent { Date: Tue, 15 Jul 2025 21:58:49 +0200 Subject: [PATCH 13/31] remove `cancelQRSignRequest` usage --- .../UI/QRHardware/QRSigningDetails.tsx | 19 +++++++++++++------ .../Views/TransactionsView/index.test.tsx | 1 - .../qr-hardware-context.test.tsx | 7 +------ .../confirmations/legacy/Approval/index.js | 5 +++-- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/app/components/UI/QRHardware/QRSigningDetails.tsx b/app/components/UI/QRHardware/QRSigningDetails.tsx index bc22e58cfe18..7419322cf88c 100644 --- a/app/components/UI/QRHardware/QRSigningDetails.tsx +++ b/app/components/UI/QRHardware/QRSigningDetails.tsx @@ -195,11 +195,14 @@ const QRSigningDetails = ({ return; } e.preventDefault(); - KeyringController.cancelQRSignRequest().then(() => { - navigation.dispatch(e.data.action); - }); + if (pendingScanRequest) { + Engine.getQrKeyringScanner().rejectPendingScan( + new Error('Scan canceled'), + ); + } + navigation.dispatch(e.data.action); }); - }, [KeyringController, hasSentOrCanceled, navigation]); + }, [pendingScanRequest, hasSentOrCanceled, navigation]); const resetError = () => { setErrorMessage(''); @@ -215,11 +218,15 @@ const QRSigningDetails = ({ }; const onCancel = useCallback(async () => { - await KeyringController.cancelQRSignRequest(); + if (pendingScanRequest) { + Engine.getQrKeyringScanner().rejectPendingScan( + new Error('Scan canceled'), + ); + } setSentOrCanceled(true); hideScanner(); cancelCallback?.(); - }, [KeyringController, cancelCallback]); + }, [pendingScanRequest, cancelCallback]); const onScanSuccess = useCallback( (ur: UR) => { diff --git a/app/components/Views/TransactionsView/index.test.tsx b/app/components/Views/TransactionsView/index.test.tsx index 9ee5b20128a6..5f1083b88446 100644 --- a/app/components/Views/TransactionsView/index.test.tsx +++ b/app/components/Views/TransactionsView/index.test.tsx @@ -138,7 +138,6 @@ jest.mock('../../UI/Transactions', () => () => null); jest.mock('../../../core/Engine', () => ({ context: { KeyringController: { - cancelQRSignRequest: jest.fn().mockResolvedValue(undefined), state: { keyrings: [], }, diff --git a/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.test.tsx b/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.test.tsx index dbd8e55e6806..952e6a7a158f 100644 --- a/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.test.tsx +++ b/app/components/Views/confirmations/context/qr-hardware-context/qr-hardware-context.test.tsx @@ -23,11 +23,6 @@ const mockQrScanner = { }; jest.mock('../../../../../core/Engine', () => ({ - context: { - KeyringController: { - cancelQRSignRequest: jest.fn().mockResolvedValue(undefined), - }, - }, controllerMessenger: { subscribe: jest.fn(), unsubscribe: jest.fn(), @@ -137,7 +132,7 @@ describe('QRHardwareContext', () => { expect(mockQrScanner.rejectPendingScan).toHaveBeenCalledTimes(1); }); - it('passes correct value of QRState components', () => { + it('passes correct value of pendingScanRequest components', () => { createCameraSpy({ cameraError: undefined, hasCameraPermission: false }); createQRHardwareAwarenessSpy({ isQRSigningInProgress: true, diff --git a/app/components/Views/confirmations/legacy/Approval/index.js b/app/components/Views/confirmations/legacy/Approval/index.js index f25703484196..4e19c147157d 100644 --- a/app/components/Views/confirmations/legacy/Approval/index.js +++ b/app/components/Views/confirmations/legacy/Approval/index.js @@ -174,11 +174,12 @@ class Approval extends PureComponent { try { const { transactionHandled } = this.state; const { transaction, selectedAddress } = this.props; - const { KeyringController } = Engine.context; if (!transactionHandled) { if (isQRHardwareAccount(selectedAddress)) { - KeyringController.cancelQRSignRequest(); + Engine.getQrKeyringScanner().rejectPendingScan( + new Error('Transaction cancelled'), + ); } else { Engine.rejectPendingApproval( transaction?.id, From 3f48f57630a36fd04b2cf5a8a6a2241b74d37810 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 15 Jul 2025 23:00:27 +0200 Subject: [PATCH 14/31] remove `isQRSigningInProgress` from `useQRHardwareAwareness()` --- .../confirmations/components/footer/footer.test.tsx | 2 +- .../confirmations/components/footer/footer.tsx | 5 ++--- .../qr-hardware-context.test.tsx | 6 ------ .../qr-hardware-context/qr-hardware-context.tsx | 12 ++++-------- .../useQRHardwareAwareness.test.ts | 1 - .../qr-hardware-context/useQRHardwareAwareness.ts | 1 - .../confirmations/hooks/useConfirmActions.test.ts | 2 +- .../Views/confirmations/hooks/useConfirmActions.ts | 13 +++++-------- 8 files changed, 13 insertions(+), 29 deletions(-) diff --git a/app/components/Views/confirmations/components/footer/footer.test.tsx b/app/components/Views/confirmations/components/footer/footer.test.tsx index 277a355be9e1..a4a75bbab39b 100644 --- a/app/components/Views/confirmations/components/footer/footer.test.tsx +++ b/app/components/Views/confirmations/components/footer/footer.test.tsx @@ -116,7 +116,7 @@ describe('Footer', () => { it('renders confirm button text "Get Signature" if QR signing is in progress', () => { jest.spyOn(QRHardwareHook, 'useQRHardwareContext').mockReturnValue({ - isQRSigningInProgress: true, + isSigningQRObject: true, } as QRHardwareHook.QRHardwareContextType); const { getByText } = renderWithProvider(