From 83d5cd9d7c748cea0f26a78b29b7fe8e7d4f576b Mon Sep 17 00:00:00 2001 From: Gaurav Goel Date: Wed, 15 Jul 2026 14:08:47 +0700 Subject: [PATCH 01/10] feat: integrate Sentry into QR account sync receive flow --- .../Views/AddDeviceToWallet/index.tsx | 22 +- app/components/Views/QRScanner/index.tsx | 13 ++ app/core/QrSync/QrSyncController.test.ts | 32 +++ app/core/QrSync/QrSyncController.ts | 116 ++++++++++- .../completeExistingUserQrSyncImport.ts | 6 + app/core/QrSync/qrSyncTelemetry.test.ts | 191 ++++++++++++++++++ app/core/QrSync/qrSyncTelemetry.ts | 179 ++++++++++++++++ .../services/qr-sync-provisioning-service.ts | 29 ++- app/core/QrSync/types.ts | 2 + .../QrSync/useQrSyncImportNavigation.test.ts | 13 +- app/core/QrSync/useQrSyncImportNavigation.ts | 18 +- .../finalizeOnboardingCompletion.ts | 10 +- 12 files changed, 582 insertions(+), 49 deletions(-) create mode 100644 app/core/QrSync/qrSyncTelemetry.test.ts create mode 100644 app/core/QrSync/qrSyncTelemetry.ts diff --git a/app/components/Views/AddDeviceToWallet/index.tsx b/app/components/Views/AddDeviceToWallet/index.tsx index d8c22d31e8f..31d70778f7f 100644 --- a/app/components/Views/AddDeviceToWallet/index.tsx +++ b/app/components/Views/AddDeviceToWallet/index.tsx @@ -29,8 +29,8 @@ import { showAddDeviceVerificationSheet } from '../../../core/QrSync/showAddDevi import { useAddDeviceResetToInstructionsListener } from '../../../core/QrSync/useAddDeviceResetToInstructionsListener'; import { useIsQrTabSwitcherOpen } from '../../../core/QrSync/useIsQrTabSwitcherOpen'; import { useQrSyncImportNavigation } from '../../../core/QrSync/useQrSyncImportNavigation'; +import { reportQrSyncFailure } from '../../../core/QrSync/qrSyncTelemetry'; import type { AppNavigationProp } from '../../../core/NavigationService/types'; -import Logger from '../../../util/Logger'; import { selectQrSyncError, selectQrSyncIsBusy, @@ -119,10 +119,11 @@ const AddDeviceToWallet = () => { const scannedQrPayload = content ?? data.content ?? ''; submitQrPayload(scannedQrPayload).catch((err: unknown) => { - Logger.error( - err as Error, - 'AddDeviceToWallet: failed to submit scanned QR payload', - ); + reportQrSyncFailure(err, { + surface: 'scanner', + operation: 'submit_scanned_payload', + source: 'AddDeviceToWallet.onScanSuccess', + }); }); }, [submitQrPayload], @@ -150,11 +151,12 @@ const AddDeviceToWallet = () => { }, [manualQrPayload, submitQrPayload]); const triggerManualQrSubmit = useCallback(() => { - handleManualQrSubmit().catch((error: unknown) => { - Logger.error( - error as Error, - 'AddDeviceToWallet: failed to submit manual QR payload', - ); + handleManualQrSubmit().catch((submitError: unknown) => { + reportQrSyncFailure(submitError, { + surface: 'scanner', + operation: 'submit_manual_payload', + source: 'AddDeviceToWallet.triggerManualQrSubmit', + }); }); }, [handleManualQrSubmit]); diff --git a/app/components/Views/QRScanner/index.tsx b/app/components/Views/QRScanner/index.tsx index 641ffe5e0e0..3792f6c1f7b 100644 --- a/app/components/Views/QRScanner/index.tsx +++ b/app/components/Views/QRScanner/index.tsx @@ -67,6 +67,7 @@ import AddDeviceScannerRecovery, { AddDeviceScannerPermissionDenied, } from './AddDeviceScannerRecovery'; import { EXTENSION_ACCOUNT_SYNC_CONNECTION_FAILED_EVENT } from '../../../core/ExtensionAccountSync/types'; +import { reportQrSyncFailure } from '../../../core/QrSync/qrSyncTelemetry'; const sleep = (ms: number) => new Promise((resolve) => { @@ -258,6 +259,18 @@ const QRScanner = ({ if (classification !== 'valid') { shouldReadBarCodeRef.current = false; setIsCameraActive(false); + reportQrSyncFailure( + new Error(`Add-device QR scan classified as ${classification}`), + { + surface: 'scanner', + operation: 'classify_scan_content', + errorCode: + classification === 'expired' + ? 'SESSION_EXPIRED' + : 'INVALID_PAYLOAD', + source: 'QRScanner.addDevice', + }, + ); trackEvent( createEventBuilder(MetaMetricsEvents.QR_SCANNED) .addProperties({ diff --git a/app/core/QrSync/QrSyncController.test.ts b/app/core/QrSync/QrSyncController.test.ts index 7079372c966..632a3c684f1 100644 --- a/app/core/QrSync/QrSyncController.test.ts +++ b/app/core/QrSync/QrSyncController.test.ts @@ -354,6 +354,38 @@ describe('QrSyncController', () => { expect(walletClient.client.sendResponse).toHaveBeenCalledTimes(1); }); + it('terminates with GRANT_WAIT_TIMEOUT when the OTP grant deadline elapses', async () => { + jest.useFakeTimers(); + const controller = buildController(); + const walletClient = buildMockWalletClient({ deferOtpAck: true }); + + mockCreateQrSyncWalletClient.mockResolvedValue({ + sessionId: VALID_SESSION_ID, + client: walletClient.client, + }); + + const scanPromise = controller.handleScannedQrPayload( + buildValidScanPayload(), + ); + await flushPromises(); + + expect(controller.state.phase).toBe(QrSyncPhases.DISPLAYING_OTP); + + jest.advanceTimersByTime(30_000); + await flushPromises(); + + expect(controller.state.phase).toBe(QrSyncPhases.FAILED); + expect(controller.state.error).toEqual({ + code: 'GRANT_WAIT_TIMEOUT', + message: 'QR sync OTP grant wait timed out before handshake completed.', + }); + + walletClient.completeOtpHandshake(); + await scanPromise; + await flushPromises(); + jest.useRealTimers(); + }); + it('sends sync-offer once after connect completes the OTP handshake', async () => { const controller = buildController(); const walletClient = buildMockWalletClient(); diff --git a/app/core/QrSync/QrSyncController.ts b/app/core/QrSync/QrSyncController.ts index d9ab0512f30..c4e2244e1ed 100644 --- a/app/core/QrSync/QrSyncController.ts +++ b/app/core/QrSync/QrSyncController.ts @@ -33,7 +33,10 @@ import { RELAY_URL, } from './constants'; import { routeIncomingQrSyncMessage } from './services/qr-sync-message-router'; -import Logger from '../../util/Logger'; +import { + addQrSyncPhaseBreadcrumb, + reportQrSyncFailure, +} from './qrSyncTelemetry'; const metadata: StateMetadata = { phase: { @@ -112,6 +115,8 @@ export class QrSyncController extends BaseController< private sessionId: string | null = null; + private grantWaitTimeoutId: ReturnType | null = null; + constructor({ messenger, state, @@ -221,10 +226,12 @@ export class QrSyncController extends BaseController< try { this.finalizeSecretImport(); } catch (error) { - Logger.error( - error as Error, - 'QrSyncController.importRemainingSecrets finalize', - ); + reportQrSyncFailure(error, { + surface: 'import', + operation: 'import_remaining_secrets_finalize', + phase: this.state.phase, + source: 'QrSyncController.importRemainingSecrets', + }); } } @@ -312,6 +319,7 @@ export class QrSyncController extends BaseController< private readonly handleClientConnected = (): void => { // Wallet-client `connected` fires after the extension verifies OTP (handshake_ack). + this.clearGrantWaitTimeout(); this.setConnectionStatus('connected'); }; @@ -390,14 +398,30 @@ export class QrSyncController extends BaseController< private readonly handleSessionServiceEvent = (event: QrSyncServiceEvent) => { switch (event.type) { case QrSyncActionTypes.OTP_DISPLAY_GRANT: { + const from = this.state.phase; + if (from !== QrSyncPhases.DISPLAYING_OTP) { + addQrSyncPhaseBreadcrumb({ + from, + to: QrSyncPhases.DISPLAYING_OTP, + }); + } this.update((state) => { state.phase = QrSyncPhases.DISPLAYING_OTP; state.otp = event.data; state.error = null; }); + this.scheduleGrantWaitTimeout(event.data.deadline); break; } case QrSyncActionTypes.SYNC_READY: { + this.clearGrantWaitTimeout(); + const from = this.state.phase; + if (from !== QrSyncPhases.REVIEWING_IMPORT) { + addQrSyncPhaseBreadcrumb({ + from, + to: QrSyncPhases.REVIEWING_IMPORT, + }); + } this.update((state) => { state.phase = QrSyncPhases.REVIEWING_IMPORT; state.error = null; @@ -405,6 +429,14 @@ export class QrSyncController extends BaseController< break; } case QrSyncActionTypes.SYNC_COMPLETED: { + this.clearGrantWaitTimeout(); + const from = this.state.phase; + if (from !== QrSyncPhases.COMPLETED) { + addQrSyncPhaseBreadcrumb({ + from, + to: QrSyncPhases.COMPLETED, + }); + } this.update((state) => { state.phase = QrSyncPhases.COMPLETED; state.otp = null; @@ -414,6 +446,7 @@ export class QrSyncController extends BaseController< break; } case QrSyncActionTypes.SYNC_CANCEL: { + this.clearGrantWaitTimeout(); this.clearControllerState(); this.destroySession().catch(() => undefined); break; @@ -438,10 +471,17 @@ export class QrSyncController extends BaseController< }, }); + const from = this.state.phase; this.update((state) => { state.otp = null; state.phase = QrSyncPhases.AWAITING_SYNC_READY; }); + if (from !== QrSyncPhases.AWAITING_SYNC_READY) { + addQrSyncPhaseBreadcrumb({ + from, + to: QrSyncPhases.AWAITING_SYNC_READY, + }); + } } private async sendSyncCompleted(): Promise { @@ -450,11 +490,19 @@ export class QrSyncController extends BaseController< version: QrSyncMessageVersion.V1, }); + this.clearGrantWaitTimeout(); + const from = this.state.phase; this.update((state) => { state.phase = QrSyncPhases.COMPLETED; state.otp = null; state.error = null; }); + if (from !== QrSyncPhases.COMPLETED) { + addQrSyncPhaseBreadcrumb({ + from, + to: QrSyncPhases.COMPLETED, + }); + } await this.destroySession(); } @@ -482,10 +530,12 @@ export class QrSyncController extends BaseController< entropySource: primaryEntropySource, }); } catch (error) { - Logger.error( - error as Error, - 'QrSyncController.enrichPrimaryProvisioningEntry', - ); + reportQrSyncFailure(error, { + surface: 'import', + operation: 'enrich_primary_provisioning_entry', + phase: this.state.phase, + source: 'QrSyncController.enrichPrimaryProvisioningEntry', + }); } } @@ -517,7 +567,11 @@ export class QrSyncController extends BaseController< await this.client.sendResponse(message); } - private transitionTo(phase: QrSyncPhase): void { + private transitionTo(phase: QrSyncPhase, errorCode?: QrSyncErrorCode): void { + const from = this.state.phase; + if (from !== phase) { + addQrSyncPhaseBreadcrumb({ from, to: phase, errorCode }); + } this.update((state) => { state.phase = phase; }); @@ -529,12 +583,36 @@ export class QrSyncController extends BaseController< ); } + private scheduleGrantWaitTimeout(deadline: number): void { + this.clearGrantWaitTimeout(); + const delayMs = Math.max(0, deadline - Date.now()); + this.grantWaitTimeoutId = setTimeout(() => { + this.grantWaitTimeoutId = null; + if (this.state.phase !== QrSyncPhases.DISPLAYING_OTP) { + return; + } + this.terminateWithError({ + code: 'GRANT_WAIT_TIMEOUT', + message: 'QR sync OTP grant wait timed out before handshake completed.', + }); + }, delayMs); + } + + private clearGrantWaitTimeout(): void { + if (this.grantWaitTimeoutId !== null) { + clearTimeout(this.grantWaitTimeoutId); + this.grantWaitTimeoutId = null; + } + } + private async notifyPeerAndEndSession( wireType: | typeof QrSyncActionTypes.SYNC_CANCEL | typeof QrSyncActionTypes.SYNC_ERROR, error?: QrSyncError, ): Promise { + this.clearGrantWaitTimeout(); + if (this.client) { try { if (wireType === QrSyncActionTypes.SYNC_ERROR && error) { @@ -557,6 +635,21 @@ export class QrSyncController extends BaseController< await this.destroySession(); if (wireType === QrSyncActionTypes.SYNC_ERROR && error) { + const from = this.state.phase; + if (from !== QrSyncPhases.FAILED) { + addQrSyncPhaseBreadcrumb({ + from, + to: QrSyncPhases.FAILED, + errorCode: error.code, + }); + } + reportQrSyncFailure(new Error(error.message), { + surface: error.code === 'GRANT_WAIT_TIMEOUT' ? 'grant_wait' : 'session', + operation: 'terminate_with_error', + errorCode: error.code, + phase: from, + source: 'QrSyncController', + }); this.update((state) => { state.phase = QrSyncPhases.FAILED; state.error = error; @@ -568,6 +661,8 @@ export class QrSyncController extends BaseController< } private async destroySession(): Promise { + this.clearGrantWaitTimeout(); + if (!this.client) { return; } @@ -616,6 +711,7 @@ export class QrSyncController extends BaseController< } private clearControllerState(): void { + this.clearGrantWaitTimeout(); this.update(() => ({ ...defaultQrSyncControllerState, })); diff --git a/app/core/QrSync/completeExistingUserQrSyncImport.ts b/app/core/QrSync/completeExistingUserQrSyncImport.ts index 7f86b6eca3e..9c6270fe6af 100644 --- a/app/core/QrSync/completeExistingUserQrSyncImport.ts +++ b/app/core/QrSync/completeExistingUserQrSyncImport.ts @@ -5,6 +5,7 @@ import { showImportFailedSheet } from '../../components/Views/AddDeviceToWallet/ import Engine from '../Engine'; import type { AppNavigationProp } from '../NavigationService/types'; import { isDuplicateMnemonicError } from './duplicateMnemonicError'; +import { reportQrSyncFailure } from './qrSyncTelemetry'; export { DUPLICATE_MNEMONIC_ERROR_MESSAGES, @@ -37,6 +38,11 @@ const runExistingUserQrSyncImport = async ( return; } + reportQrSyncFailure(error, { + surface: 'import', + operation: 'existing_user_mnemonic_import', + source: 'completeExistingUserQrSyncImport', + }); navigation.navigate(Routes.WALLET_VIEW); showImportFailedSheet(navigation); } diff --git a/app/core/QrSync/qrSyncTelemetry.test.ts b/app/core/QrSync/qrSyncTelemetry.test.ts new file mode 100644 index 00000000000..0bc77081cd0 --- /dev/null +++ b/app/core/QrSync/qrSyncTelemetry.test.ts @@ -0,0 +1,191 @@ +import { addBreadcrumb } from '@sentry/react-native'; + +import Logger from '../../util/Logger'; +import { + QR_SYNC_SENTRY_FEATURE, + addQrSyncPhaseBreadcrumb, + buildQrSyncLoggerErrorOptions, + reportQrSyncFailure, + scrubSensitiveQrSyncData, +} from './qrSyncTelemetry'; + +jest.mock('@sentry/react-native', () => ({ + addBreadcrumb: jest.fn(), +})); + +jest.mock('../../util/Logger', () => ({ + __esModule: true, + default: { + error: jest.fn(), + log: jest.fn(), + }, +})); + +const TEST_MNEMONIC = + 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about'; +const TEST_ADDRESS = '0x742d35Cc6634C0532925a3b844Bc9e7595f0bEb0'; +const TEST_OTP = '123456'; +const TEST_MWP_DEEPLINK = + 'metamask://connect/mwp?p=eyJzZXNzaW9uUmVxdWVzdCI6eyJpZCI6InRlc3QifX0'; + +describe('qrSyncTelemetry', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('scrubSensitiveQrSyncData', () => { + it('redacts sensitive object keys', () => { + const scrubbed = scrubSensitiveQrSyncData({ + otp: TEST_OTP, + mnemonic: TEST_MNEMONIC, + privateKey: '0xabc', + pendingSecretImports: [{ value: TEST_MNEMONIC }], + phase: 'displaying_otp', + }) as Record; + + expect(scrubbed.otp).toBe('[REDACTED]'); + expect(scrubbed.mnemonic).toBe('[REDACTED]'); + expect(scrubbed.privateKey).toBe('[REDACTED]'); + expect(scrubbed.pendingSecretImports).toBe('[REDACTED]'); + expect(scrubbed.phase).toBe('displaying_otp'); + }); + + it('redacts addresses, mnemonics, and deeplinks in strings', () => { + const scrubbed = scrubSensitiveQrSyncData( + `fail ${TEST_ADDRESS} seed ${TEST_MNEMONIC} link ${TEST_MWP_DEEPLINK}`, + ) as string; + + expect(scrubbed).not.toContain(TEST_ADDRESS); + expect(scrubbed).not.toContain(TEST_MNEMONIC); + expect(scrubbed).not.toContain(TEST_MWP_DEEPLINK); + expect(scrubbed).toContain('[REDACTED]'); + }); + + it('redacts nested values without dropping safe fields', () => { + const scrubbed = scrubSensitiveQrSyncData({ + surface: 'scanner', + extras: { + content: TEST_MWP_DEEPLINK, + note: 'ok', + }, + }) as { + surface: string; + extras: { content: string; note: string }; + }; + + expect(scrubbed.surface).toBe('scanner'); + expect(scrubbed.extras.note).toBe('ok'); + expect(scrubbed.extras.content).toBe('[REDACTED]'); + }); + }); + + describe('addQrSyncPhaseBreadcrumb', () => { + it('emits a phase breadcrumb without secrets', () => { + addQrSyncPhaseBreadcrumb({ + from: 'initializing', + to: 'displaying_otp', + }); + + expect(addBreadcrumb).toHaveBeenCalledWith({ + category: 'qr_sync', + level: 'info', + message: 'qr_sync.phase initializing->displaying_otp', + data: { + from: 'initializing', + to: 'displaying_otp', + }, + }); + }); + + it('marks failed transitions as error level with errorCode', () => { + addQrSyncPhaseBreadcrumb({ + from: 'displaying_otp', + to: 'failed', + errorCode: 'GRANT_WAIT_TIMEOUT', + }); + + expect(addBreadcrumb).toHaveBeenCalledWith( + expect.objectContaining({ + level: 'error', + message: + 'qr_sync.phase displaying_otp->failed code=GRANT_WAIT_TIMEOUT', + data: expect.objectContaining({ + errorCode: 'GRANT_WAIT_TIMEOUT', + }), + }), + ); + }); + }); + + describe('buildQrSyncLoggerErrorOptions', () => { + it('sets feature:qr-sync tags and scrubs injected secrets from extras', () => { + const options = buildQrSyncLoggerErrorOptions({ + surface: 'import', + operation: 'existing_user_import', + error: new Error('vault import failed'), + errorCode: 'SYNC_FAILED', + phase: 'reviewing_import', + source: 'completeExistingUserQrSyncImport', + extras: { + mnemonic: TEST_MNEMONIC, + otp: TEST_OTP, + address: TEST_ADDRESS, + scanContent: TEST_MWP_DEEPLINK, + }, + }); + + expect(options.tags).toEqual( + expect.objectContaining({ + feature: QR_SYNC_SENTRY_FEATURE, + surface: 'import', + operation: 'existing_user_import', + errorCode: 'SYNC_FAILED', + }), + ); + expect(JSON.stringify(options.extras)).not.toContain(TEST_MNEMONIC); + expect(JSON.stringify(options.extras)).not.toContain(TEST_OTP); + expect(JSON.stringify(options.extras)).not.toContain(TEST_ADDRESS); + expect(JSON.stringify(options.extras)).not.toContain(TEST_MWP_DEEPLINK); + expect(JSON.stringify(options.context?.data)).not.toContain( + TEST_MNEMONIC, + ); + }); + }); + + describe('reportQrSyncFailure', () => { + it('forwards scrubbed LoggerErrorOptions to Logger.error', () => { + reportQrSyncFailure(new Error('scan submit failed'), { + surface: 'scanner', + operation: 'submit_scanned_payload', + source: 'AddDeviceToWallet', + extras: { qrPayload: TEST_MWP_DEEPLINK }, + }); + + expect(Logger.error).toHaveBeenCalledTimes(1); + const [, options] = jest.mocked(Logger.error).mock.calls[0]; + expect(options).toEqual( + expect.objectContaining({ + tags: expect.objectContaining({ + feature: QR_SYNC_SENTRY_FEATURE, + surface: 'scanner', + }), + }), + ); + expect(JSON.stringify(options)).not.toContain(TEST_MWP_DEEPLINK); + }); + + it('wraps non-Error values', () => { + reportQrSyncFailure('boom', { + surface: 'session', + operation: 'terminate', + }); + + expect(Logger.error).toHaveBeenCalledWith( + expect.any(Error), + expect.objectContaining({ + tags: expect.objectContaining({ feature: QR_SYNC_SENTRY_FEATURE }), + }), + ); + }); + }); +}); diff --git a/app/core/QrSync/qrSyncTelemetry.ts b/app/core/QrSync/qrSyncTelemetry.ts new file mode 100644 index 00000000000..50d74f4d8bc --- /dev/null +++ b/app/core/QrSync/qrSyncTelemetry.ts @@ -0,0 +1,179 @@ +/** + * Sentry observability helpers for the QR sync receive flow (ADR-0055). + * + * Tags: `feature:qr-sync` — queryable like `feature:social` / `feature:perps`. + * Breadcrumbs and failure extras must never include OTP, SRP, addresses, or raw QR payloads. + */ + +import { addBreadcrumb } from '@sentry/react-native'; + +import Logger, { type LoggerErrorOptions } from '../../util/Logger'; +import type { QrSyncErrorCode, QrSyncPhase } from './types'; + +/** Sentry tag value for QR sync errors. Query: `feature:qr-sync`. */ +export const QR_SYNC_SENTRY_FEATURE = 'qr-sync' as const; + +/** UI / protocol surface within the QR sync receive flow. */ +export type QrSyncSurface = 'scanner' | 'grant_wait' | 'import' | 'session'; + +const SENSITIVE_KEY_PATTERN = + /^(otp|mnemonic|seed|seedPhrase|privateKey|pendingSecretImports|value|p|payload|scannedQr|qrPayload|sessionRequest)$/i; + +const ETH_ADDRESS_PATTERN = /0x[a-fA-F0-9]{40}/g; +const MWP_DEEPLINK_PATTERN = /metamask:\/\/connect\/mwp\?[^\s"']+/gi; +const E2E_QR_SYNC_DEEPLINK_PATTERN = + /(?:metamask:\/\/e2e\/qr-sync\/|e2e:\/\/qr-sync\/)[^\s"']*/gi; +/** Heuristic for BIP39-like mnemonic blobs in free-form strings. */ +const MNEMONIC_BLOB_PATTERN = /\b(?:[a-z]+(?:\s+[a-z]+){11,23})\b/gi; + +const REDACTED = '[REDACTED]'; + +export interface QrSyncPhaseBreadcrumbArgs { + from: QrSyncPhase; + to: QrSyncPhase; + errorCode?: QrSyncErrorCode | string; +} + +export interface ReportQrSyncFailureOptions { + surface: QrSyncSurface; + operation: string; + errorCode?: QrSyncErrorCode | string; + phase?: QrSyncPhase | string; + source?: string; + /** Extra display-only fields — scrubbed before send. */ + extras?: Record; +} + +/** + * Deep-scrub sensitive QR sync fields before attaching to Sentry extras/context. + */ +export function scrubSensitiveQrSyncData(value: unknown): unknown { + if (value === null || value === undefined) { + return value; + } + + if (typeof value === 'string') { + return scrubSensitiveString(value); + } + + if (typeof value === 'number' || typeof value === 'boolean') { + return value; + } + + if (Array.isArray(value)) { + return value.map((entry) => scrubSensitiveQrSyncData(entry)); + } + + if (typeof value === 'object') { + const result: Record = {}; + for (const [key, entry] of Object.entries( + value as Record, + )) { + if (SENSITIVE_KEY_PATTERN.test(key)) { + result[key] = REDACTED; + continue; + } + result[key] = scrubSensitiveQrSyncData(entry); + } + return result; + } + + return String(value); +} + +function scrubSensitiveString(raw: string): string { + return raw + .replace(MWP_DEEPLINK_PATTERN, REDACTED) + .replace(E2E_QR_SYNC_DEEPLINK_PATTERN, REDACTED) + .replace(ETH_ADDRESS_PATTERN, REDACTED) + .replace(MNEMONIC_BLOB_PATTERN, REDACTED); +} + +/** + * Phase-transition breadcrumb without secrets (from/to/errorCode only). + */ +export function addQrSyncPhaseBreadcrumb({ + from, + to, + errorCode, +}: QrSyncPhaseBreadcrumbArgs): void { + const parts = [`qr_sync.phase ${from}->${to}`]; + if (errorCode !== undefined) { + parts.push(`code=${errorCode}`); + } + + addBreadcrumb({ + category: 'qr_sync', + level: to === 'failed' ? 'error' : 'info', + message: parts.join(' '), + data: { + from, + to, + ...(errorCode !== undefined && { errorCode }), + }, + }); +} + +/** + * Build searchable Logger.error options for QR sync failures. + */ +export function buildQrSyncLoggerErrorOptions({ + surface, + operation, + error, + errorCode, + phase, + source, + extras, +}: ReportQrSyncFailureOptions & { error: unknown }): LoggerErrorOptions { + const errorMessage = + error instanceof Error ? error.message : String(error ?? ''); + + const scrubbedExtras = scrubSensitiveQrSyncData({ + message: `qr-sync ${surface}.${operation}`, + errorMessage, + ...(errorCode !== undefined && { errorCode }), + ...(phase !== undefined && { phase }), + ...(source !== undefined && { source }), + ...(extras !== undefined && extras), + }) as Record; + + return { + tags: { + feature: QR_SYNC_SENTRY_FEATURE, + surface, + operation, + ...(errorCode !== undefined && { errorCode: String(errorCode) }), + ...(phase !== undefined && { phase: String(phase) }), + ...(source !== undefined && { source }), + }, + context: { + name: 'qr_sync', + data: scrubSensitiveQrSyncData({ + surface, + operation, + ...(errorCode !== undefined && { errorCode }), + ...(phase !== undefined && { phase }), + ...(source !== undefined && { source }), + }) as Record, + }, + extras: scrubbedExtras, + }; +} + +/** + * Report a QR sync failure to Sentry with `feature:qr-sync` tags and scrubbed extras. + */ +export function reportQrSyncFailure( + error: unknown, + options: ReportQrSyncFailureOptions, +): void { + const err = error instanceof Error ? error : new Error(String(error ?? '')); + Logger.error( + err, + buildQrSyncLoggerErrorOptions({ + ...options, + error, + }), + ); +} diff --git a/app/core/QrSync/services/qr-sync-provisioning-service.ts b/app/core/QrSync/services/qr-sync-provisioning-service.ts index 87e18ba5a22..e69ba4b22ff 100644 --- a/app/core/QrSync/services/qr-sync-provisioning-service.ts +++ b/app/core/QrSync/services/qr-sync-provisioning-service.ts @@ -49,7 +49,7 @@ import type { QrSyncSecretImportEntry, } from '../types'; import { toFormattedAddress } from '../../../util/address'; -import Logger from '../../../util/Logger'; +import { reportQrSyncFailure } from '../qrSyncTelemetry'; const SERVICE_NAME = 'QrSyncProvisioningService' as const; @@ -147,16 +147,22 @@ export class QrSyncProvisioningService { ); } } else { - Logger.error( + reportQrSyncFailure( new Error('QrSyncProvisioningService: Unknown secret type'), - secret.type, + { + surface: 'import', + operation: 'import_secrets_unknown_type', + source: 'QrSyncProvisioningService.importSecretsToVault', + extras: { secretType: String(secret.type) }, + }, ); } } catch (error) { - Logger.error( - error as Error, - 'QrSyncProvisioningService.importSecretsToVault', - ); + reportQrSyncFailure(error, { + surface: 'import', + operation: 'import_secrets_to_vault', + source: 'QrSyncProvisioningService.importSecretsToVault', + }); } } } @@ -426,10 +432,11 @@ export class QrSyncProvisioningService { try { await this.#messenger.call('AccountTreeController:syncWithUserStorage'); } catch (error) { - Logger.error( - error as Error, - 'QrSyncProvisioningService: user storage reconciliation failed', - ); + reportQrSyncFailure(error, { + surface: 'import', + operation: 'user_storage_reconciliation', + source: 'QrSyncProvisioningService.reconcileWithUserStorage', + }); } } diff --git a/app/core/QrSync/types.ts b/app/core/QrSync/types.ts index ab95fbcfb04..23d81e0332b 100644 --- a/app/core/QrSync/types.ts +++ b/app/core/QrSync/types.ts @@ -51,6 +51,7 @@ export interface QrSyncOffer { * - `INVALID_PAYLOAD` — scan payload, wire message, or import data failed validation * - `UNSUPPORTED_VERSION` — peer message uses an unsupported protocol version * - `SESSION_EXPIRED` — scanned session request or sync-ready deadline has expired + * - `GRANT_WAIT_TIMEOUT` — OTP was shown but handshake did not complete before the grant deadline * - `SYNC_REJECTED` — extension explicitly rejected the sync (peer-originated) * - `SYNC_FAILED` — unexpected runtime or wallet-client failure during an active session */ @@ -60,6 +61,7 @@ export type QrSyncErrorCode = | 'INVALID_PAYLOAD' | 'UNSUPPORTED_VERSION' | 'SESSION_EXPIRED' + | 'GRANT_WAIT_TIMEOUT' | 'SYNC_REJECTED' | 'SYNC_FAILED' | 'UNKNOWN_ERROR'; diff --git a/app/core/QrSync/useQrSyncImportNavigation.test.ts b/app/core/QrSync/useQrSyncImportNavigation.test.ts index 14b379daf71..9bcca7cba28 100644 --- a/app/core/QrSync/useQrSyncImportNavigation.test.ts +++ b/app/core/QrSync/useQrSyncImportNavigation.test.ts @@ -78,12 +78,12 @@ jest.mock( }), ); -jest.mock('../../util/Logger', () => ({ - error: jest.fn(), - log: jest.fn(), +jest.mock('./qrSyncTelemetry', () => ({ + reportQrSyncFailure: jest.fn(), })); import Engine from '../Engine'; +import { reportQrSyncFailure } from './qrSyncTelemetry'; const flushAsync = async () => { await waitFor(() => { @@ -299,10 +299,7 @@ describe('useQrSyncImportNavigation', () => { expect(mockImportRemainingSecrets).not.toHaveBeenCalled(); }); - it('logs and resets when existing-user mnemonic import rejects', async () => { - const Logger = jest.requireMock('../../util/Logger') as { - error: jest.Mock; - }; + it('reports and resets when existing-user mnemonic import rejects', async () => { mockCompletedOnboarding = true; mockShouldNavigateToImport = true; mockQrSyncMnemonic = 'seed from redux'; @@ -314,7 +311,7 @@ describe('useQrSyncImportNavigation', () => { renderHook(() => useQrSyncImportNavigation({ enabled: true })); await waitFor(() => { - expect(Logger.error).toHaveBeenCalled(); + expect(reportQrSyncFailure).toHaveBeenCalled(); expect(mockResetState).toHaveBeenCalled(); }); }); diff --git a/app/core/QrSync/useQrSyncImportNavigation.ts b/app/core/QrSync/useQrSyncImportNavigation.ts index 8122eb1b919..531a3596561 100644 --- a/app/core/QrSync/useQrSyncImportNavigation.ts +++ b/app/core/QrSync/useQrSyncImportNavigation.ts @@ -16,7 +16,7 @@ import { navigateToQrSyncImport } from './navigateToQrSyncImport'; import { showAlreadySyncedSheet } from '../../components/Views/AddDeviceToWallet/showAlreadySyncedSheet'; import { showImportFailedSheet } from '../../components/Views/AddDeviceToWallet/showImportFailedSheet'; import type { QrSyncSecretImportEntry } from './types'; -import Logger from '../../util/Logger'; +import { reportQrSyncFailure } from './qrSyncTelemetry'; interface UseQrSyncImportNavigationOptions { enabled: boolean; @@ -57,8 +57,13 @@ const finishExistingUserSyncWithoutMnemonic = async ( try { await Engine.context.QrSyncController.importRemainingSecrets(); - } catch { + } catch (error) { importFailed = true; + reportQrSyncFailure(error, { + surface: 'import', + operation: 'import_remaining_secrets', + source: 'finishExistingUserSyncWithoutMnemonic', + }); } const accountsAfter = await Engine.context.KeyringController.getAccounts(); @@ -125,10 +130,11 @@ export const useQrSyncImportNavigation = ({ .catch((error: unknown) => { hasHandledImportNavigationRef.current = false; Engine.context.QrSyncController.resetState(); - Logger.error( - error as Error, - 'useQrSyncImportNavigation: existing-user import failed', - ); + reportQrSyncFailure(error, { + surface: 'import', + operation: 'existing_user_import_navigation', + source: 'useQrSyncImportNavigation', + }); }) .finally(() => { inFlightImportNavigation = null; diff --git a/app/util/onboarding/finalizeOnboardingCompletion.ts b/app/util/onboarding/finalizeOnboardingCompletion.ts index 918708d5071..c22bb12a454 100644 --- a/app/util/onboarding/finalizeOnboardingCompletion.ts +++ b/app/util/onboarding/finalizeOnboardingCompletion.ts @@ -10,6 +10,7 @@ import { getOnboardingCompletedAnalyticsPropsFromSuccessFlow } from '../analytic import type { WalletSetupCompletedAttributionAnalyticsPayload } from '../analytics/walletSetupCompletedAttribution'; import { analytics } from '../analytics/analytics'; import Logger from '../Logger'; +import { reportQrSyncFailure } from '../../core/QrSync/qrSyncTelemetry'; import { shouldMarkWalletHomeOnboardingStepsEligible } from './walletHomeOnboardingStepsEligibility'; export interface FinalizeOnboardingCompletionParams { @@ -74,10 +75,11 @@ export function finalizeOnboardingCompletion({ if (needsQrProvisioning) { Engine.context.QrSyncProvisioningService.provisionFromMetadata().catch( (error: unknown) => { - Logger.error( - error as Error, - `${discoverAccountsLogContext}: provisionFromMetadata failed`, - ); + reportQrSyncFailure(error, { + surface: 'import', + operation: 'provision_from_metadata', + source: discoverAccountsLogContext, + }); }, ); } else if (keyrings?.length > 0) { From 77ac9c92ea44a48d65bc577f32226e9b716d3731 Mon Sep 17 00:00:00 2001 From: Gaurav Goel Date: Wed, 15 Jul 2026 14:47:26 +0700 Subject: [PATCH 02/10] fix: lint error and unit-tests --- .../OptinMetrics/OptinMetrics.navigation.test.tsx | 9 ++++++++- app/core/QrSync/QrSyncController.test.ts | 12 ++++++++++++ app/core/QrSync/QrSyncController.ts | 14 ++++++++++++++ app/core/QrSync/qrSyncTelemetry.test.ts | 10 +++++----- 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx b/app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx index 6709da09542..3c0b5c95a9b 100644 --- a/app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx +++ b/app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx @@ -505,7 +505,14 @@ describe('OptinMetrics — interest questionnaire navigation branching', () => { await waitFor(() => { expect(Logger.error).toHaveBeenCalledWith( expect.objectContaining({ message: 'provisioning failed' }), - 'OptinMetrics: provisionFromMetadata failed', + expect.objectContaining({ + tags: expect.objectContaining({ + feature: 'qr-sync', + surface: 'import', + operation: 'provision_from_metadata', + source: 'OptinMetrics', + }), + }), ); }); }); diff --git a/app/core/QrSync/QrSyncController.test.ts b/app/core/QrSync/QrSyncController.test.ts index 632a3c684f1..b0a3f4ec5e1 100644 --- a/app/core/QrSync/QrSyncController.test.ts +++ b/app/core/QrSync/QrSyncController.test.ts @@ -383,6 +383,18 @@ describe('QrSyncController', () => { walletClient.completeOtpHandshake(); await scanPromise; await flushPromises(); + + expect(controller.state.phase).toBe(QrSyncPhases.FAILED); + expect(controller.state.error?.code).toBe('GRANT_WAIT_TIMEOUT'); + expect(walletClient.client.sendResponse).toHaveBeenCalledWith( + expect.objectContaining({ + type: QrSyncActionTypes.SYNC_ERROR, + data: expect.objectContaining({ code: 'GRANT_WAIT_TIMEOUT' }), + }), + ); + expect(walletClient.client.sendResponse).not.toHaveBeenCalledWith( + expect.objectContaining({ type: QrSyncActionTypes.SYNC_OFFER }), + ); jest.useRealTimers(); }); diff --git a/app/core/QrSync/QrSyncController.ts b/app/core/QrSync/QrSyncController.ts index c4e2244e1ed..74af2a52567 100644 --- a/app/core/QrSync/QrSyncController.ts +++ b/app/core/QrSync/QrSyncController.ts @@ -173,8 +173,14 @@ export class QrSyncController extends BaseController< this.attachClient(client, sessionId); this.setConnectionStatus('connecting'); await client.connect({ sessionRequest }); + if (this.state.phase === QrSyncPhases.FAILED) { + return; + } await this.sendSyncOffer(); } catch (error) { + if (this.state.phase === QrSyncPhases.FAILED) { + return; + } this.terminateWithError(this.toQrSyncError(error, 'CHANNEL_INIT_FAILED')); } } @@ -462,6 +468,10 @@ export class QrSyncController extends BaseController< }; private async sendSyncOffer(): Promise { + if (this.state.phase === QrSyncPhases.FAILED) { + return; + } + await this.sendMessage({ type: QrSyncActionTypes.SYNC_OFFER, version: QrSyncMessageVersion.V1, @@ -471,6 +481,10 @@ export class QrSyncController extends BaseController< }, }); + if (this.state.phase === QrSyncPhases.FAILED) { + return; + } + const from = this.state.phase; this.update((state) => { state.otp = null; diff --git a/app/core/QrSync/qrSyncTelemetry.test.ts b/app/core/QrSync/qrSyncTelemetry.test.ts index 0bc77081cd0..4821821ecad 100644 --- a/app/core/QrSync/qrSyncTelemetry.test.ts +++ b/app/core/QrSync/qrSyncTelemetry.test.ts @@ -83,23 +83,23 @@ describe('qrSyncTelemetry', () => { it('emits a phase breadcrumb without secrets', () => { addQrSyncPhaseBreadcrumb({ from: 'initializing', - to: 'displaying_otp', + to: 'displaying-otp', }); expect(addBreadcrumb).toHaveBeenCalledWith({ category: 'qr_sync', level: 'info', - message: 'qr_sync.phase initializing->displaying_otp', + message: 'qr_sync.phase initializing->displaying-otp', data: { from: 'initializing', - to: 'displaying_otp', + to: 'displaying-otp', }, }); }); it('marks failed transitions as error level with errorCode', () => { addQrSyncPhaseBreadcrumb({ - from: 'displaying_otp', + from: 'displaying-otp', to: 'failed', errorCode: 'GRANT_WAIT_TIMEOUT', }); @@ -108,7 +108,7 @@ describe('qrSyncTelemetry', () => { expect.objectContaining({ level: 'error', message: - 'qr_sync.phase displaying_otp->failed code=GRANT_WAIT_TIMEOUT', + 'qr_sync.phase displaying-otp->failed code=GRANT_WAIT_TIMEOUT', data: expect.objectContaining({ errorCode: 'GRANT_WAIT_TIMEOUT', }), From 602cbe0016363156caaba71cc2db517fe9e86a06 Mon Sep 17 00:00:00 2001 From: Gaurav Goel Date: Wed, 15 Jul 2026 15:00:37 +0700 Subject: [PATCH 03/10] fix: lint issues --- app/core/QrSync/QrSyncController.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/core/QrSync/QrSyncController.ts b/app/core/QrSync/QrSyncController.ts index 74af2a52567..0ad43e56c9a 100644 --- a/app/core/QrSync/QrSyncController.ts +++ b/app/core/QrSync/QrSyncController.ts @@ -173,12 +173,12 @@ export class QrSyncController extends BaseController< this.attachClient(client, sessionId); this.setConnectionStatus('connecting'); await client.connect({ sessionRequest }); - if (this.state.phase === QrSyncPhases.FAILED) { + if (this.isFailedPhase()) { return; } await this.sendSyncOffer(); } catch (error) { - if (this.state.phase === QrSyncPhases.FAILED) { + if (this.isFailedPhase()) { return; } this.terminateWithError(this.toQrSyncError(error, 'CHANNEL_INIT_FAILED')); @@ -468,7 +468,7 @@ export class QrSyncController extends BaseController< }; private async sendSyncOffer(): Promise { - if (this.state.phase === QrSyncPhases.FAILED) { + if (this.isFailedPhase()) { return; } @@ -481,7 +481,7 @@ export class QrSyncController extends BaseController< }, }); - if (this.state.phase === QrSyncPhases.FAILED) { + if (this.isFailedPhase()) { return; } @@ -591,6 +591,10 @@ export class QrSyncController extends BaseController< }); } + private isFailedPhase(): boolean { + return this.state.phase === QrSyncPhases.FAILED; + } + private terminateWithError(error: QrSyncError): void { this.notifyPeerAndEndSession(QrSyncActionTypes.SYNC_ERROR, error).catch( () => undefined, From 15fecfc9fb1fb4753146b6e611505eb506499ab0 Mon Sep 17 00:00:00 2001 From: Gaurav Goel Date: Wed, 15 Jul 2026 15:34:06 +0700 Subject: [PATCH 04/10] fix: unit tests --- app/util/onboarding/finalizeOnboardingCompletion.test.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/util/onboarding/finalizeOnboardingCompletion.test.ts b/app/util/onboarding/finalizeOnboardingCompletion.test.ts index a0f98bab706..dc298236618 100644 --- a/app/util/onboarding/finalizeOnboardingCompletion.test.ts +++ b/app/util/onboarding/finalizeOnboardingCompletion.test.ts @@ -147,7 +147,14 @@ describe('finalizeOnboardingCompletion', () => { expect(loggerSpy).toHaveBeenCalledWith( expect.any(Error), - 'TestContext: provisionFromMetadata failed', + expect.objectContaining({ + tags: expect.objectContaining({ + feature: 'qr-sync', + surface: 'import', + operation: 'provision_from_metadata', + source: 'TestContext', + }), + }), ); expect(mockDispatch).toHaveBeenCalledWith(clearAttribution()); From d5c2002587c1d4e639e43bcd2fe55bb1e6cbf583 Mon Sep 17 00:00:00 2001 From: Gaurav Goel Date: Wed, 15 Jul 2026 16:05:22 +0700 Subject: [PATCH 05/10] address PR comments --- .../OptinMetrics.navigation.test.tsx | 1 + .../Views/AddDeviceToWallet/index.tsx | 19 +++-- app/components/Views/QRScanner/index.tsx | 30 ++++---- app/core/QrSync/QrSyncController.test.ts | 44 ----------- app/core/QrSync/QrSyncController.ts | 74 +++---------------- .../completeExistingUserQrSyncImport.ts | 15 +++- app/core/QrSync/qrSyncTelemetry.test.ts | 41 +++++----- app/core/QrSync/qrSyncTelemetry.ts | 66 ++++++++++++++++- .../services/qr-sync-provisioning-service.ts | 51 ++++++++++--- app/core/QrSync/types.ts | 2 - .../QrSync/useQrSyncImportNavigation.test.ts | 10 ++- app/core/QrSync/useQrSyncImportNavigation.ts | 22 ++++-- .../finalizeOnboardingCompletion.test.ts | 1 + .../finalizeOnboardingCompletion.ts | 13 +++- 14 files changed, 212 insertions(+), 177 deletions(-) diff --git a/app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx b/app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx index 3c0b5c95a9b..b2fd578d5f0 100644 --- a/app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx +++ b/app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx @@ -511,6 +511,7 @@ describe('OptinMetrics — interest questionnaire navigation branching', () => { surface: 'import', operation: 'provision_from_metadata', source: 'OptinMetrics', + syncFlow: 'new_user', }), }), ); diff --git a/app/components/Views/AddDeviceToWallet/index.tsx b/app/components/Views/AddDeviceToWallet/index.tsx index 31d70778f7f..5bd25bdada5 100644 --- a/app/components/Views/AddDeviceToWallet/index.tsx +++ b/app/components/Views/AddDeviceToWallet/index.tsx @@ -29,7 +29,12 @@ import { showAddDeviceVerificationSheet } from '../../../core/QrSync/showAddDevi import { useAddDeviceResetToInstructionsListener } from '../../../core/QrSync/useAddDeviceResetToInstructionsListener'; import { useIsQrTabSwitcherOpen } from '../../../core/QrSync/useIsQrTabSwitcherOpen'; import { useQrSyncImportNavigation } from '../../../core/QrSync/useQrSyncImportNavigation'; -import { reportQrSyncFailure } from '../../../core/QrSync/qrSyncTelemetry'; +import { + QrSyncOperations, + QrSyncSurfaces, + QrSyncTelemetrySources, + reportQrSyncFailure, +} from '../../../core/QrSync/qrSyncTelemetry'; import type { AppNavigationProp } from '../../../core/NavigationService/types'; import { selectQrSyncError, @@ -120,9 +125,9 @@ const AddDeviceToWallet = () => { submitQrPayload(scannedQrPayload).catch((err: unknown) => { reportQrSyncFailure(err, { - surface: 'scanner', - operation: 'submit_scanned_payload', - source: 'AddDeviceToWallet.onScanSuccess', + surface: QrSyncSurfaces.SCANNER, + operation: QrSyncOperations.SUBMIT_SCANNED_PAYLOAD, + source: QrSyncTelemetrySources.ADD_DEVICE_ON_SCAN_SUCCESS, }); }); }, @@ -153,9 +158,9 @@ const AddDeviceToWallet = () => { const triggerManualQrSubmit = useCallback(() => { handleManualQrSubmit().catch((submitError: unknown) => { reportQrSyncFailure(submitError, { - surface: 'scanner', - operation: 'submit_manual_payload', - source: 'AddDeviceToWallet.triggerManualQrSubmit', + surface: QrSyncSurfaces.SCANNER, + operation: QrSyncOperations.SUBMIT_MANUAL_PAYLOAD, + source: QrSyncTelemetrySources.ADD_DEVICE_MANUAL_SUBMIT, }); }); }, [handleManualQrSubmit]); diff --git a/app/components/Views/QRScanner/index.tsx b/app/components/Views/QRScanner/index.tsx index 3792f6c1f7b..c838bbdde02 100644 --- a/app/components/Views/QRScanner/index.tsx +++ b/app/components/Views/QRScanner/index.tsx @@ -67,7 +67,12 @@ import AddDeviceScannerRecovery, { AddDeviceScannerPermissionDenied, } from './AddDeviceScannerRecovery'; import { EXTENSION_ACCOUNT_SYNC_CONNECTION_FAILED_EVENT } from '../../../core/ExtensionAccountSync/types'; -import { reportQrSyncFailure } from '../../../core/QrSync/qrSyncTelemetry'; +import { + QrSyncOperations, + QrSyncSurfaces, + QrSyncTelemetrySources, + reportQrSyncFailure, +} from '../../../core/QrSync/qrSyncTelemetry'; const sleep = (ms: number) => new Promise((resolve) => { @@ -259,18 +264,17 @@ const QRScanner = ({ if (classification !== 'valid') { shouldReadBarCodeRef.current = false; setIsCameraActive(false); - reportQrSyncFailure( - new Error(`Add-device QR scan classified as ${classification}`), - { - surface: 'scanner', - operation: 'classify_scan_content', - errorCode: - classification === 'expired' - ? 'SESSION_EXPIRED' - : 'INVALID_PAYLOAD', - source: 'QRScanner.addDevice', - }, - ); + if (classification === 'invalid') { + reportQrSyncFailure( + new Error('Add-device QR scan classified as invalid'), + { + surface: QrSyncSurfaces.SCANNER, + operation: QrSyncOperations.CLASSIFY_SCAN_CONTENT, + errorCode: 'INVALID_PAYLOAD', + source: QrSyncTelemetrySources.QR_SCANNER_ADD_DEVICE, + }, + ); + } trackEvent( createEventBuilder(MetaMetricsEvents.QR_SCANNED) .addProperties({ diff --git a/app/core/QrSync/QrSyncController.test.ts b/app/core/QrSync/QrSyncController.test.ts index b0a3f4ec5e1..7079372c966 100644 --- a/app/core/QrSync/QrSyncController.test.ts +++ b/app/core/QrSync/QrSyncController.test.ts @@ -354,50 +354,6 @@ describe('QrSyncController', () => { expect(walletClient.client.sendResponse).toHaveBeenCalledTimes(1); }); - it('terminates with GRANT_WAIT_TIMEOUT when the OTP grant deadline elapses', async () => { - jest.useFakeTimers(); - const controller = buildController(); - const walletClient = buildMockWalletClient({ deferOtpAck: true }); - - mockCreateQrSyncWalletClient.mockResolvedValue({ - sessionId: VALID_SESSION_ID, - client: walletClient.client, - }); - - const scanPromise = controller.handleScannedQrPayload( - buildValidScanPayload(), - ); - await flushPromises(); - - expect(controller.state.phase).toBe(QrSyncPhases.DISPLAYING_OTP); - - jest.advanceTimersByTime(30_000); - await flushPromises(); - - expect(controller.state.phase).toBe(QrSyncPhases.FAILED); - expect(controller.state.error).toEqual({ - code: 'GRANT_WAIT_TIMEOUT', - message: 'QR sync OTP grant wait timed out before handshake completed.', - }); - - walletClient.completeOtpHandshake(); - await scanPromise; - await flushPromises(); - - expect(controller.state.phase).toBe(QrSyncPhases.FAILED); - expect(controller.state.error?.code).toBe('GRANT_WAIT_TIMEOUT'); - expect(walletClient.client.sendResponse).toHaveBeenCalledWith( - expect.objectContaining({ - type: QrSyncActionTypes.SYNC_ERROR, - data: expect.objectContaining({ code: 'GRANT_WAIT_TIMEOUT' }), - }), - ); - expect(walletClient.client.sendResponse).not.toHaveBeenCalledWith( - expect.objectContaining({ type: QrSyncActionTypes.SYNC_OFFER }), - ); - jest.useRealTimers(); - }); - it('sends sync-offer once after connect completes the OTP handshake', async () => { const controller = buildController(); const walletClient = buildMockWalletClient(); diff --git a/app/core/QrSync/QrSyncController.ts b/app/core/QrSync/QrSyncController.ts index 0ad43e56c9a..592891065e8 100644 --- a/app/core/QrSync/QrSyncController.ts +++ b/app/core/QrSync/QrSyncController.ts @@ -35,6 +35,9 @@ import { import { routeIncomingQrSyncMessage } from './services/qr-sync-message-router'; import { addQrSyncPhaseBreadcrumb, + QrSyncOperations, + QrSyncSurfaces, + QrSyncTelemetrySources, reportQrSyncFailure, } from './qrSyncTelemetry'; @@ -115,8 +118,6 @@ export class QrSyncController extends BaseController< private sessionId: string | null = null; - private grantWaitTimeoutId: ReturnType | null = null; - constructor({ messenger, state, @@ -173,14 +174,8 @@ export class QrSyncController extends BaseController< this.attachClient(client, sessionId); this.setConnectionStatus('connecting'); await client.connect({ sessionRequest }); - if (this.isFailedPhase()) { - return; - } await this.sendSyncOffer(); } catch (error) { - if (this.isFailedPhase()) { - return; - } this.terminateWithError(this.toQrSyncError(error, 'CHANNEL_INIT_FAILED')); } } @@ -233,10 +228,10 @@ export class QrSyncController extends BaseController< this.finalizeSecretImport(); } catch (error) { reportQrSyncFailure(error, { - surface: 'import', - operation: 'import_remaining_secrets_finalize', + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.IMPORT_REMAINING_SECRETS_FINALIZE, phase: this.state.phase, - source: 'QrSyncController.importRemainingSecrets', + source: QrSyncTelemetrySources.CONTROLLER_IMPORT_REMAINING, }); } } @@ -325,7 +320,6 @@ export class QrSyncController extends BaseController< private readonly handleClientConnected = (): void => { // Wallet-client `connected` fires after the extension verifies OTP (handshake_ack). - this.clearGrantWaitTimeout(); this.setConnectionStatus('connected'); }; @@ -416,11 +410,9 @@ export class QrSyncController extends BaseController< state.otp = event.data; state.error = null; }); - this.scheduleGrantWaitTimeout(event.data.deadline); break; } case QrSyncActionTypes.SYNC_READY: { - this.clearGrantWaitTimeout(); const from = this.state.phase; if (from !== QrSyncPhases.REVIEWING_IMPORT) { addQrSyncPhaseBreadcrumb({ @@ -435,7 +427,6 @@ export class QrSyncController extends BaseController< break; } case QrSyncActionTypes.SYNC_COMPLETED: { - this.clearGrantWaitTimeout(); const from = this.state.phase; if (from !== QrSyncPhases.COMPLETED) { addQrSyncPhaseBreadcrumb({ @@ -452,7 +443,6 @@ export class QrSyncController extends BaseController< break; } case QrSyncActionTypes.SYNC_CANCEL: { - this.clearGrantWaitTimeout(); this.clearControllerState(); this.destroySession().catch(() => undefined); break; @@ -468,10 +458,6 @@ export class QrSyncController extends BaseController< }; private async sendSyncOffer(): Promise { - if (this.isFailedPhase()) { - return; - } - await this.sendMessage({ type: QrSyncActionTypes.SYNC_OFFER, version: QrSyncMessageVersion.V1, @@ -481,10 +467,6 @@ export class QrSyncController extends BaseController< }, }); - if (this.isFailedPhase()) { - return; - } - const from = this.state.phase; this.update((state) => { state.otp = null; @@ -504,7 +486,6 @@ export class QrSyncController extends BaseController< version: QrSyncMessageVersion.V1, }); - this.clearGrantWaitTimeout(); const from = this.state.phase; this.update((state) => { state.phase = QrSyncPhases.COMPLETED; @@ -545,10 +526,10 @@ export class QrSyncController extends BaseController< }); } catch (error) { reportQrSyncFailure(error, { - surface: 'import', - operation: 'enrich_primary_provisioning_entry', + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.ENRICH_PRIMARY_PROVISIONING_ENTRY, phase: this.state.phase, - source: 'QrSyncController.enrichPrimaryProvisioningEntry', + source: QrSyncTelemetrySources.CONTROLLER_ENRICH_PRIMARY, }); } } @@ -591,46 +572,18 @@ export class QrSyncController extends BaseController< }); } - private isFailedPhase(): boolean { - return this.state.phase === QrSyncPhases.FAILED; - } - private terminateWithError(error: QrSyncError): void { this.notifyPeerAndEndSession(QrSyncActionTypes.SYNC_ERROR, error).catch( () => undefined, ); } - private scheduleGrantWaitTimeout(deadline: number): void { - this.clearGrantWaitTimeout(); - const delayMs = Math.max(0, deadline - Date.now()); - this.grantWaitTimeoutId = setTimeout(() => { - this.grantWaitTimeoutId = null; - if (this.state.phase !== QrSyncPhases.DISPLAYING_OTP) { - return; - } - this.terminateWithError({ - code: 'GRANT_WAIT_TIMEOUT', - message: 'QR sync OTP grant wait timed out before handshake completed.', - }); - }, delayMs); - } - - private clearGrantWaitTimeout(): void { - if (this.grantWaitTimeoutId !== null) { - clearTimeout(this.grantWaitTimeoutId); - this.grantWaitTimeoutId = null; - } - } - private async notifyPeerAndEndSession( wireType: | typeof QrSyncActionTypes.SYNC_CANCEL | typeof QrSyncActionTypes.SYNC_ERROR, error?: QrSyncError, ): Promise { - this.clearGrantWaitTimeout(); - if (this.client) { try { if (wireType === QrSyncActionTypes.SYNC_ERROR && error) { @@ -662,11 +615,11 @@ export class QrSyncController extends BaseController< }); } reportQrSyncFailure(new Error(error.message), { - surface: error.code === 'GRANT_WAIT_TIMEOUT' ? 'grant_wait' : 'session', - operation: 'terminate_with_error', + surface: QrSyncSurfaces.SESSION, + operation: QrSyncOperations.TERMINATE_WITH_ERROR, errorCode: error.code, phase: from, - source: 'QrSyncController', + source: QrSyncTelemetrySources.CONTROLLER, }); this.update((state) => { state.phase = QrSyncPhases.FAILED; @@ -679,8 +632,6 @@ export class QrSyncController extends BaseController< } private async destroySession(): Promise { - this.clearGrantWaitTimeout(); - if (!this.client) { return; } @@ -729,7 +680,6 @@ export class QrSyncController extends BaseController< } private clearControllerState(): void { - this.clearGrantWaitTimeout(); this.update(() => ({ ...defaultQrSyncControllerState, })); diff --git a/app/core/QrSync/completeExistingUserQrSyncImport.ts b/app/core/QrSync/completeExistingUserQrSyncImport.ts index 9c6270fe6af..95841c29b2c 100644 --- a/app/core/QrSync/completeExistingUserQrSyncImport.ts +++ b/app/core/QrSync/completeExistingUserQrSyncImport.ts @@ -5,7 +5,13 @@ import { showImportFailedSheet } from '../../components/Views/AddDeviceToWallet/ import Engine from '../Engine'; import type { AppNavigationProp } from '../NavigationService/types'; import { isDuplicateMnemonicError } from './duplicateMnemonicError'; -import { reportQrSyncFailure } from './qrSyncTelemetry'; +import { + QrSyncOperations, + QrSyncSurfaces, + QrSyncSyncFlows, + QrSyncTelemetrySources, + reportQrSyncFailure, +} from './qrSyncTelemetry'; export { DUPLICATE_MNEMONIC_ERROR_MESSAGES, @@ -39,9 +45,10 @@ const runExistingUserQrSyncImport = async ( } reportQrSyncFailure(error, { - surface: 'import', - operation: 'existing_user_mnemonic_import', - source: 'completeExistingUserQrSyncImport', + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.EXISTING_USER_MNEMONIC_IMPORT, + source: QrSyncTelemetrySources.COMPLETE_EXISTING_USER_IMPORT, + syncFlow: QrSyncSyncFlows.EXISTING_USER, }); navigation.navigate(Routes.WALLET_VIEW); showImportFailedSheet(navigation); diff --git a/app/core/QrSync/qrSyncTelemetry.test.ts b/app/core/QrSync/qrSyncTelemetry.test.ts index 4821821ecad..d523889b326 100644 --- a/app/core/QrSync/qrSyncTelemetry.test.ts +++ b/app/core/QrSync/qrSyncTelemetry.test.ts @@ -3,6 +3,9 @@ import { addBreadcrumb } from '@sentry/react-native'; import Logger from '../../util/Logger'; import { QR_SYNC_SENTRY_FEATURE, + QrSyncOperations, + QrSyncSurfaces, + QrSyncTelemetrySources, addQrSyncPhaseBreadcrumb, buildQrSyncLoggerErrorOptions, reportQrSyncFailure, @@ -40,14 +43,14 @@ describe('qrSyncTelemetry', () => { mnemonic: TEST_MNEMONIC, privateKey: '0xabc', pendingSecretImports: [{ value: TEST_MNEMONIC }], - phase: 'displaying_otp', + phase: 'displaying-otp', }) as Record; expect(scrubbed.otp).toBe('[REDACTED]'); expect(scrubbed.mnemonic).toBe('[REDACTED]'); expect(scrubbed.privateKey).toBe('[REDACTED]'); expect(scrubbed.pendingSecretImports).toBe('[REDACTED]'); - expect(scrubbed.phase).toBe('displaying_otp'); + expect(scrubbed.phase).toBe('displaying-otp'); }); it('redacts addresses, mnemonics, and deeplinks in strings', () => { @@ -63,7 +66,7 @@ describe('qrSyncTelemetry', () => { it('redacts nested values without dropping safe fields', () => { const scrubbed = scrubSensitiveQrSyncData({ - surface: 'scanner', + surface: QrSyncSurfaces.SCANNER, extras: { content: TEST_MWP_DEEPLINK, note: 'ok', @@ -73,7 +76,7 @@ describe('qrSyncTelemetry', () => { extras: { content: string; note: string }; }; - expect(scrubbed.surface).toBe('scanner'); + expect(scrubbed.surface).toBe(QrSyncSurfaces.SCANNER); expect(scrubbed.extras.note).toBe('ok'); expect(scrubbed.extras.content).toBe('[REDACTED]'); }); @@ -101,16 +104,16 @@ describe('qrSyncTelemetry', () => { addQrSyncPhaseBreadcrumb({ from: 'displaying-otp', to: 'failed', - errorCode: 'GRANT_WAIT_TIMEOUT', + errorCode: 'CHANNEL_DISCONNECTED', }); expect(addBreadcrumb).toHaveBeenCalledWith( expect.objectContaining({ level: 'error', message: - 'qr_sync.phase displaying-otp->failed code=GRANT_WAIT_TIMEOUT', + 'qr_sync.phase displaying-otp->failed code=CHANNEL_DISCONNECTED', data: expect.objectContaining({ - errorCode: 'GRANT_WAIT_TIMEOUT', + errorCode: 'CHANNEL_DISCONNECTED', }), }), ); @@ -120,12 +123,12 @@ describe('qrSyncTelemetry', () => { describe('buildQrSyncLoggerErrorOptions', () => { it('sets feature:qr-sync tags and scrubs injected secrets from extras', () => { const options = buildQrSyncLoggerErrorOptions({ - surface: 'import', - operation: 'existing_user_import', + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.EXISTING_USER_MNEMONIC_IMPORT, error: new Error('vault import failed'), errorCode: 'SYNC_FAILED', - phase: 'reviewing_import', - source: 'completeExistingUserQrSyncImport', + phase: 'reviewing-import', + source: QrSyncTelemetrySources.COMPLETE_EXISTING_USER_IMPORT, extras: { mnemonic: TEST_MNEMONIC, otp: TEST_OTP, @@ -137,8 +140,8 @@ describe('qrSyncTelemetry', () => { expect(options.tags).toEqual( expect.objectContaining({ feature: QR_SYNC_SENTRY_FEATURE, - surface: 'import', - operation: 'existing_user_import', + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.EXISTING_USER_MNEMONIC_IMPORT, errorCode: 'SYNC_FAILED', }), ); @@ -155,9 +158,9 @@ describe('qrSyncTelemetry', () => { describe('reportQrSyncFailure', () => { it('forwards scrubbed LoggerErrorOptions to Logger.error', () => { reportQrSyncFailure(new Error('scan submit failed'), { - surface: 'scanner', - operation: 'submit_scanned_payload', - source: 'AddDeviceToWallet', + surface: QrSyncSurfaces.SCANNER, + operation: QrSyncOperations.SUBMIT_SCANNED_PAYLOAD, + source: QrSyncTelemetrySources.ADD_DEVICE_ON_SCAN_SUCCESS, extras: { qrPayload: TEST_MWP_DEEPLINK }, }); @@ -167,7 +170,7 @@ describe('qrSyncTelemetry', () => { expect.objectContaining({ tags: expect.objectContaining({ feature: QR_SYNC_SENTRY_FEATURE, - surface: 'scanner', + surface: QrSyncSurfaces.SCANNER, }), }), ); @@ -176,8 +179,8 @@ describe('qrSyncTelemetry', () => { it('wraps non-Error values', () => { reportQrSyncFailure('boom', { - surface: 'session', - operation: 'terminate', + surface: QrSyncSurfaces.SESSION, + operation: QrSyncOperations.TERMINATE_WITH_ERROR, }); expect(Logger.error).toHaveBeenCalledWith( diff --git a/app/core/QrSync/qrSyncTelemetry.ts b/app/core/QrSync/qrSyncTelemetry.ts index 50d74f4d8bc..c433db502fa 100644 --- a/app/core/QrSync/qrSyncTelemetry.ts +++ b/app/core/QrSync/qrSyncTelemetry.ts @@ -14,7 +14,62 @@ import type { QrSyncErrorCode, QrSyncPhase } from './types'; export const QR_SYNC_SENTRY_FEATURE = 'qr-sync' as const; /** UI / protocol surface within the QR sync receive flow. */ -export type QrSyncSurface = 'scanner' | 'grant_wait' | 'import' | 'session'; +export const QrSyncSurfaces = { + SCANNER: 'scanner', + IMPORT: 'import', + SESSION: 'session', +} as const; + +export type QrSyncSurface = + (typeof QrSyncSurfaces)[keyof typeof QrSyncSurfaces]; + +/** Operation identifiers attached to `feature:qr-sync` Sentry events. */ +export const QrSyncOperations = { + CLASSIFY_SCAN_CONTENT: 'classify_scan_content', + SUBMIT_SCANNED_PAYLOAD: 'submit_scanned_payload', + SUBMIT_MANUAL_PAYLOAD: 'submit_manual_payload', + EXISTING_USER_MNEMONIC_IMPORT: 'existing_user_mnemonic_import', + IMPORT_REMAINING_SECRETS: 'import_remaining_secrets', + EXISTING_USER_IMPORT_NAVIGATION: 'existing_user_import_navigation', + PROVISION_FROM_METADATA: 'provision_from_metadata', + IMPORT_SECRETS_UNKNOWN_TYPE: 'import_secrets_unknown_type', + IMPORT_SECRETS_TO_VAULT: 'import_secrets_to_vault', + USER_STORAGE_RECONCILIATION: 'user_storage_reconciliation', + IMPORT_REMAINING_SECRETS_FINALIZE: 'import_remaining_secrets_finalize', + ENRICH_PRIMARY_PROVISIONING_ENTRY: 'enrich_primary_provisioning_entry', + TERMINATE_WITH_ERROR: 'terminate_with_error', +} as const; + +export type QrSyncOperation = + (typeof QrSyncOperations)[keyof typeof QrSyncOperations]; + +/** Stable `source` tags for QR sync failure call sites. */ +export const QrSyncTelemetrySources = { + QR_SCANNER_ADD_DEVICE: 'QRScanner.addDevice', + ADD_DEVICE_ON_SCAN_SUCCESS: 'AddDeviceToWallet.onScanSuccess', + ADD_DEVICE_MANUAL_SUBMIT: 'AddDeviceToWallet.triggerManualQrSubmit', + COMPLETE_EXISTING_USER_IMPORT: 'completeExistingUserQrSyncImport', + FINISH_EXISTING_USER_WITHOUT_MNEMONIC: + 'finishExistingUserSyncWithoutMnemonic', + USE_QR_SYNC_IMPORT_NAVIGATION: 'useQrSyncImportNavigation', + PROVISIONING_IMPORT_SECRETS: 'QrSyncProvisioningService.importSecretsToVault', + PROVISIONING_RECONCILE: 'QrSyncProvisioningService.reconcileWithUserStorage', + CONTROLLER_IMPORT_REMAINING: 'QrSyncController.importRemainingSecrets', + CONTROLLER_ENRICH_PRIMARY: 'QrSyncController.enrichPrimaryProvisioningEntry', + CONTROLLER: 'QrSyncController', +} as const; + +export type QrSyncTelemetrySource = + (typeof QrSyncTelemetrySources)[keyof typeof QrSyncTelemetrySources]; + +/** Distinguishes new-user vs existing-user QR sync receive paths. */ +export const QrSyncSyncFlows = { + NEW_USER: 'new_user', + EXISTING_USER: 'existing_user', +} as const; + +export type QrSyncSyncFlow = + (typeof QrSyncSyncFlows)[keyof typeof QrSyncSyncFlows]; const SENSITIVE_KEY_PATTERN = /^(otp|mnemonic|seed|seedPhrase|privateKey|pendingSecretImports|value|p|payload|scannedQr|qrPayload|sessionRequest)$/i; @@ -36,10 +91,11 @@ export interface QrSyncPhaseBreadcrumbArgs { export interface ReportQrSyncFailureOptions { surface: QrSyncSurface; - operation: string; + operation: QrSyncOperation; errorCode?: QrSyncErrorCode | string; phase?: QrSyncPhase | string; - source?: string; + source?: QrSyncTelemetrySource | string; + syncFlow?: QrSyncSyncFlow; /** Extra display-only fields — scrubbed before send. */ extras?: Record; } @@ -124,6 +180,7 @@ export function buildQrSyncLoggerErrorOptions({ errorCode, phase, source, + syncFlow, extras, }: ReportQrSyncFailureOptions & { error: unknown }): LoggerErrorOptions { const errorMessage = @@ -135,6 +192,7 @@ export function buildQrSyncLoggerErrorOptions({ ...(errorCode !== undefined && { errorCode }), ...(phase !== undefined && { phase }), ...(source !== undefined && { source }), + ...(syncFlow !== undefined && { syncFlow }), ...(extras !== undefined && extras), }) as Record; @@ -146,6 +204,7 @@ export function buildQrSyncLoggerErrorOptions({ ...(errorCode !== undefined && { errorCode: String(errorCode) }), ...(phase !== undefined && { phase: String(phase) }), ...(source !== undefined && { source }), + ...(syncFlow !== undefined && { syncFlow }), }, context: { name: 'qr_sync', @@ -155,6 +214,7 @@ export function buildQrSyncLoggerErrorOptions({ ...(errorCode !== undefined && { errorCode }), ...(phase !== undefined && { phase }), ...(source !== undefined && { source }), + ...(syncFlow !== undefined && { syncFlow }), }) as Record, }, extras: scrubbedExtras, diff --git a/app/core/QrSync/services/qr-sync-provisioning-service.ts b/app/core/QrSync/services/qr-sync-provisioning-service.ts index e69ba4b22ff..d68f36e5989 100644 --- a/app/core/QrSync/services/qr-sync-provisioning-service.ts +++ b/app/core/QrSync/services/qr-sync-provisioning-service.ts @@ -49,7 +49,13 @@ import type { QrSyncSecretImportEntry, } from '../types'; import { toFormattedAddress } from '../../../util/address'; -import { reportQrSyncFailure } from '../qrSyncTelemetry'; +import { + QrSyncOperations, + QrSyncSurfaces, + QrSyncSyncFlows, + QrSyncTelemetrySources, + reportQrSyncFailure, +} from '../qrSyncTelemetry'; const SERVICE_NAME = 'QrSyncProvisioningService' as const; @@ -147,21 +153,24 @@ export class QrSyncProvisioningService { ); } } else { + const syncFlow = this.#resolveSyncFlow(); reportQrSyncFailure( new Error('QrSyncProvisioningService: Unknown secret type'), { - surface: 'import', - operation: 'import_secrets_unknown_type', - source: 'QrSyncProvisioningService.importSecretsToVault', + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.IMPORT_SECRETS_UNKNOWN_TYPE, + source: QrSyncTelemetrySources.PROVISIONING_IMPORT_SECRETS, + syncFlow, extras: { secretType: String(secret.type) }, }, ); } } catch (error) { reportQrSyncFailure(error, { - surface: 'import', - operation: 'import_secrets_to_vault', - source: 'QrSyncProvisioningService.importSecretsToVault', + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.IMPORT_SECRETS_TO_VAULT, + source: QrSyncTelemetrySources.PROVISIONING_IMPORT_SECRETS, + syncFlow: this.#resolveSyncFlow(), }); } } @@ -433,13 +442,35 @@ export class QrSyncProvisioningService { await this.#messenger.call('AccountTreeController:syncWithUserStorage'); } catch (error) { reportQrSyncFailure(error, { - surface: 'import', - operation: 'user_storage_reconciliation', - source: 'QrSyncProvisioningService.reconcileWithUserStorage', + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.USER_STORAGE_RECONCILIATION, + source: QrSyncTelemetrySources.PROVISIONING_RECONCILE, + syncFlow: this.#resolveSyncFlow(), }); } } + /** + * Best-effort new vs existing-user classification for Sentry tags. + * Pending primary mnemonic usually indicates the new-user onboarding path. + */ + #resolveSyncFlow(): + | typeof QrSyncSyncFlows.NEW_USER + | typeof QrSyncSyncFlows.EXISTING_USER { + try { + const { pendingSecretImports } = this.#getQrSyncControllerState(); + const hasPrimaryPendingMnemonic = pendingSecretImports?.some( + (secret) => + secret.type === QrSyncSecretTypes.MNEMONIC && secret.isPrimary, + ); + return hasPrimaryPendingMnemonic + ? QrSyncSyncFlows.NEW_USER + : QrSyncSyncFlows.EXISTING_USER; + } catch { + return QrSyncSyncFlows.EXISTING_USER; + } + } + #getAccountWalletObjects(): AccountWalletObject[] { return this.#messenger.call( 'AccountTreeController:getAccountWalletObjects', diff --git a/app/core/QrSync/types.ts b/app/core/QrSync/types.ts index 23d81e0332b..ab95fbcfb04 100644 --- a/app/core/QrSync/types.ts +++ b/app/core/QrSync/types.ts @@ -51,7 +51,6 @@ export interface QrSyncOffer { * - `INVALID_PAYLOAD` — scan payload, wire message, or import data failed validation * - `UNSUPPORTED_VERSION` — peer message uses an unsupported protocol version * - `SESSION_EXPIRED` — scanned session request or sync-ready deadline has expired - * - `GRANT_WAIT_TIMEOUT` — OTP was shown but handshake did not complete before the grant deadline * - `SYNC_REJECTED` — extension explicitly rejected the sync (peer-originated) * - `SYNC_FAILED` — unexpected runtime or wallet-client failure during an active session */ @@ -61,7 +60,6 @@ export type QrSyncErrorCode = | 'INVALID_PAYLOAD' | 'UNSUPPORTED_VERSION' | 'SESSION_EXPIRED' - | 'GRANT_WAIT_TIMEOUT' | 'SYNC_REJECTED' | 'SYNC_FAILED' | 'UNKNOWN_ERROR'; diff --git a/app/core/QrSync/useQrSyncImportNavigation.test.ts b/app/core/QrSync/useQrSyncImportNavigation.test.ts index 9bcca7cba28..615be60cb2e 100644 --- a/app/core/QrSync/useQrSyncImportNavigation.test.ts +++ b/app/core/QrSync/useQrSyncImportNavigation.test.ts @@ -78,9 +78,13 @@ jest.mock( }), ); -jest.mock('./qrSyncTelemetry', () => ({ - reportQrSyncFailure: jest.fn(), -})); +jest.mock('./qrSyncTelemetry', () => { + const actual = jest.requireActual('./qrSyncTelemetry'); + return { + ...actual, + reportQrSyncFailure: jest.fn(), + }; +}); import Engine from '../Engine'; import { reportQrSyncFailure } from './qrSyncTelemetry'; diff --git a/app/core/QrSync/useQrSyncImportNavigation.ts b/app/core/QrSync/useQrSyncImportNavigation.ts index 531a3596561..58962b78b34 100644 --- a/app/core/QrSync/useQrSyncImportNavigation.ts +++ b/app/core/QrSync/useQrSyncImportNavigation.ts @@ -16,7 +16,13 @@ import { navigateToQrSyncImport } from './navigateToQrSyncImport'; import { showAlreadySyncedSheet } from '../../components/Views/AddDeviceToWallet/showAlreadySyncedSheet'; import { showImportFailedSheet } from '../../components/Views/AddDeviceToWallet/showImportFailedSheet'; import type { QrSyncSecretImportEntry } from './types'; -import { reportQrSyncFailure } from './qrSyncTelemetry'; +import { + QrSyncOperations, + QrSyncSurfaces, + QrSyncSyncFlows, + QrSyncTelemetrySources, + reportQrSyncFailure, +} from './qrSyncTelemetry'; interface UseQrSyncImportNavigationOptions { enabled: boolean; @@ -60,9 +66,10 @@ const finishExistingUserSyncWithoutMnemonic = async ( } catch (error) { importFailed = true; reportQrSyncFailure(error, { - surface: 'import', - operation: 'import_remaining_secrets', - source: 'finishExistingUserSyncWithoutMnemonic', + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.IMPORT_REMAINING_SECRETS, + source: QrSyncTelemetrySources.FINISH_EXISTING_USER_WITHOUT_MNEMONIC, + syncFlow: QrSyncSyncFlows.EXISTING_USER, }); } @@ -131,9 +138,10 @@ export const useQrSyncImportNavigation = ({ hasHandledImportNavigationRef.current = false; Engine.context.QrSyncController.resetState(); reportQrSyncFailure(error, { - surface: 'import', - operation: 'existing_user_import_navigation', - source: 'useQrSyncImportNavigation', + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.EXISTING_USER_IMPORT_NAVIGATION, + source: QrSyncTelemetrySources.USE_QR_SYNC_IMPORT_NAVIGATION, + syncFlow: QrSyncSyncFlows.EXISTING_USER, }); }) .finally(() => { diff --git a/app/util/onboarding/finalizeOnboardingCompletion.test.ts b/app/util/onboarding/finalizeOnboardingCompletion.test.ts index dc298236618..c05cf614f9d 100644 --- a/app/util/onboarding/finalizeOnboardingCompletion.test.ts +++ b/app/util/onboarding/finalizeOnboardingCompletion.test.ts @@ -153,6 +153,7 @@ describe('finalizeOnboardingCompletion', () => { surface: 'import', operation: 'provision_from_metadata', source: 'TestContext', + syncFlow: 'new_user', }), }), ); diff --git a/app/util/onboarding/finalizeOnboardingCompletion.ts b/app/util/onboarding/finalizeOnboardingCompletion.ts index c22bb12a454..574ee801a86 100644 --- a/app/util/onboarding/finalizeOnboardingCompletion.ts +++ b/app/util/onboarding/finalizeOnboardingCompletion.ts @@ -10,7 +10,13 @@ import { getOnboardingCompletedAnalyticsPropsFromSuccessFlow } from '../analytic import type { WalletSetupCompletedAttributionAnalyticsPayload } from '../analytics/walletSetupCompletedAttribution'; import { analytics } from '../analytics/analytics'; import Logger from '../Logger'; -import { reportQrSyncFailure } from '../../core/QrSync/qrSyncTelemetry'; +import { + QrSyncOperations, + QrSyncSurfaces, + QrSyncSyncFlows, + QrSyncTelemetrySources, + reportQrSyncFailure, +} from '../../core/QrSync/qrSyncTelemetry'; import { shouldMarkWalletHomeOnboardingStepsEligible } from './walletHomeOnboardingStepsEligibility'; export interface FinalizeOnboardingCompletionParams { @@ -76,9 +82,10 @@ export function finalizeOnboardingCompletion({ Engine.context.QrSyncProvisioningService.provisionFromMetadata().catch( (error: unknown) => { reportQrSyncFailure(error, { - surface: 'import', - operation: 'provision_from_metadata', + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.PROVISION_FROM_METADATA, source: discoverAccountsLogContext, + syncFlow: QrSyncSyncFlows.NEW_USER, }); }, ); From 23185b6e359edd326118b9a8774ea36d111ce4aa Mon Sep 17 00:00:00 2001 From: Gaurav Goel Date: Wed, 15 Jul 2026 23:01:19 +0700 Subject: [PATCH 06/10] address PR comments --- app/components/Views/QRScanner/index.tsx | 3 +- app/core/QrSync/QrSyncController.test.ts | 2 + app/core/QrSync/QrSyncController.ts | 145 ++++++++---------- app/core/QrSync/constants.ts | 9 ++ app/core/QrSync/controller-types.ts | 2 + app/core/QrSync/qrSyncTelemetry.test.ts | 14 +- app/core/QrSync/qrSyncTelemetry.ts | 37 ++--- .../services/qr-sync-provisioning-service.ts | 31 ++-- app/core/QrSync/types.ts | 20 ++- 9 files changed, 129 insertions(+), 134 deletions(-) diff --git a/app/components/Views/QRScanner/index.tsx b/app/components/Views/QRScanner/index.tsx index c838bbdde02..1add8d00cb5 100644 --- a/app/components/Views/QRScanner/index.tsx +++ b/app/components/Views/QRScanner/index.tsx @@ -73,6 +73,7 @@ import { QrSyncTelemetrySources, reportQrSyncFailure, } from '../../../core/QrSync/qrSyncTelemetry'; +import { QrSyncErrorCodes } from '../../../core/QrSync/types'; const sleep = (ms: number) => new Promise((resolve) => { @@ -270,7 +271,7 @@ const QRScanner = ({ { surface: QrSyncSurfaces.SCANNER, operation: QrSyncOperations.CLASSIFY_SCAN_CONTENT, - errorCode: 'INVALID_PAYLOAD', + errorCode: QrSyncErrorCodes.INVALID_PAYLOAD, source: QrSyncTelemetrySources.QR_SCANNER_ADD_DEVICE, }, ); diff --git a/app/core/QrSync/QrSyncController.test.ts b/app/core/QrSync/QrSyncController.test.ts index 7079372c966..0013fa0ac3d 100644 --- a/app/core/QrSync/QrSyncController.test.ts +++ b/app/core/QrSync/QrSyncController.test.ts @@ -12,6 +12,7 @@ import { QrSyncPhases, QrSyncProvisioningStatuses, QrSyncSecretTypes, + QrSyncSyncFlows, } from './constants'; import { QR_SYNC_CONTROLLER_NAME, @@ -239,6 +240,7 @@ describe('QrSyncController', () => { expect(walletClient.client.sendResponse).toHaveBeenCalledTimes(1); expect(controller.state.phase).toBe(QrSyncPhases.AWAITING_SYNC_READY); expect(controller.state.connectionStatus).toBe('connected'); + expect(controller.state.syncFlow).toBe(QrSyncSyncFlows.EXISTING_USER); }); it('throws when the scan payload cannot be parsed', async () => { diff --git a/app/core/QrSync/QrSyncController.ts b/app/core/QrSync/QrSyncController.ts index 592891065e8..5521fe261ac 100644 --- a/app/core/QrSync/QrSyncController.ts +++ b/app/core/QrSync/QrSyncController.ts @@ -30,6 +30,7 @@ import { QrSyncPhases, QrSyncProvisioningStatuses, QrSyncSecretTypes, + QrSyncSyncFlows, RELAY_URL, } from './constants'; import { routeIncomingQrSyncMessage } from './services/qr-sync-message-router'; @@ -54,6 +55,12 @@ const metadata: StateMetadata = { includeInStateLogs: true, usedInUi: true, }, + syncFlow: { + persist: false, + includeInDebugSnapshot: true, + includeInStateLogs: true, + usedInUi: false, + }, otp: { persist: false, includeInDebugSnapshot: false, @@ -89,6 +96,7 @@ const metadata: StateMetadata = { export const defaultQrSyncControllerState: QrSyncControllerState = { phase: QrSyncPhases.IDLE, connectionStatus: 'disconnected', + syncFlow: null, pendingSecretImports: null, provisioningMetadata: null, provisioningStatus: null, @@ -160,6 +168,12 @@ export class QrSyncController extends BaseController< // Destroy any existing session before starting a new one. await this.destroySession(); this.clearControllerState(); + // Capture sync flow once from local onboarding status at session start. + this.update((state) => { + state.syncFlow = this.getIsOnboardingCompleted() + ? QrSyncSyncFlows.EXISTING_USER + : QrSyncSyncFlows.NEW_USER; + }); this.transitionTo(QrSyncPhases.INITIALIZING); try { @@ -397,61 +411,37 @@ export class QrSyncController extends BaseController< private readonly handleSessionServiceEvent = (event: QrSyncServiceEvent) => { switch (event.type) { - case QrSyncActionTypes.OTP_DISPLAY_GRANT: { - const from = this.state.phase; - if (from !== QrSyncPhases.DISPLAYING_OTP) { - addQrSyncPhaseBreadcrumb({ - from, - to: QrSyncPhases.DISPLAYING_OTP, - }); - } - this.update((state) => { - state.phase = QrSyncPhases.DISPLAYING_OTP; - state.otp = event.data; - state.error = null; + case QrSyncActionTypes.OTP_DISPLAY_GRANT: + this.transitionTo(QrSyncPhases.DISPLAYING_OTP, { + patch: (state) => { + state.otp = event.data; + state.error = null; + }, }); break; - } - case QrSyncActionTypes.SYNC_READY: { - const from = this.state.phase; - if (from !== QrSyncPhases.REVIEWING_IMPORT) { - addQrSyncPhaseBreadcrumb({ - from, - to: QrSyncPhases.REVIEWING_IMPORT, - }); - } - this.update((state) => { - state.phase = QrSyncPhases.REVIEWING_IMPORT; - state.error = null; + case QrSyncActionTypes.SYNC_READY: + this.transitionTo(QrSyncPhases.REVIEWING_IMPORT, { + patch: (state) => { + state.error = null; + }, }); break; - } - case QrSyncActionTypes.SYNC_COMPLETED: { - const from = this.state.phase; - if (from !== QrSyncPhases.COMPLETED) { - addQrSyncPhaseBreadcrumb({ - from, - to: QrSyncPhases.COMPLETED, - }); - } - this.update((state) => { - state.phase = QrSyncPhases.COMPLETED; - state.otp = null; - state.error = null; + case QrSyncActionTypes.SYNC_COMPLETED: + this.transitionTo(QrSyncPhases.COMPLETED, { + patch: (state) => { + state.otp = null; + state.error = null; + }, }); this.destroySession().catch(() => undefined); break; - } - case QrSyncActionTypes.SYNC_CANCEL: { + case QrSyncActionTypes.SYNC_CANCEL: this.clearControllerState(); this.destroySession().catch(() => undefined); break; - } - case QrSyncActionTypes.SYNC_ERROR: { + case QrSyncActionTypes.SYNC_ERROR: this.terminateWithError(event.data); break; - } - default: // no-op } @@ -467,17 +457,11 @@ export class QrSyncController extends BaseController< }, }); - const from = this.state.phase; - this.update((state) => { - state.otp = null; - state.phase = QrSyncPhases.AWAITING_SYNC_READY; + this.transitionTo(QrSyncPhases.AWAITING_SYNC_READY, { + patch: (state) => { + state.otp = null; + }, }); - if (from !== QrSyncPhases.AWAITING_SYNC_READY) { - addQrSyncPhaseBreadcrumb({ - from, - to: QrSyncPhases.AWAITING_SYNC_READY, - }); - } } private async sendSyncCompleted(): Promise { @@ -486,18 +470,12 @@ export class QrSyncController extends BaseController< version: QrSyncMessageVersion.V1, }); - const from = this.state.phase; - this.update((state) => { - state.phase = QrSyncPhases.COMPLETED; - state.otp = null; - state.error = null; + this.transitionTo(QrSyncPhases.COMPLETED, { + patch: (state) => { + state.otp = null; + state.error = null; + }, }); - if (from !== QrSyncPhases.COMPLETED) { - addQrSyncPhaseBreadcrumb({ - from, - to: QrSyncPhases.COMPLETED, - }); - } await this.destroySession(); } @@ -562,13 +540,24 @@ export class QrSyncController extends BaseController< await this.client.sendResponse(message); } - private transitionTo(phase: QrSyncPhase, errorCode?: QrSyncErrorCode): void { - const from = this.state.phase; - if (from !== phase) { - addQrSyncPhaseBreadcrumb({ from, to: phase, errorCode }); + private transitionTo( + phase: QrSyncPhase, + options?: { + errorCode?: QrSyncErrorCode; + patch?: (state: QrSyncControllerState) => void; + }, + ): void { + const phaseFrom = this.state.phase; + if (phaseFrom !== phase) { + addQrSyncPhaseBreadcrumb({ + phaseFrom, + phaseTo: phase, + errorCode: options?.errorCode, + }); } this.update((state) => { state.phase = phase; + options?.patch?.(state); }); } @@ -606,24 +595,20 @@ export class QrSyncController extends BaseController< await this.destroySession(); if (wireType === QrSyncActionTypes.SYNC_ERROR && error) { - const from = this.state.phase; - if (from !== QrSyncPhases.FAILED) { - addQrSyncPhaseBreadcrumb({ - from, - to: QrSyncPhases.FAILED, - errorCode: error.code, - }); - } + const phaseFrom = this.state.phase; reportQrSyncFailure(new Error(error.message), { surface: QrSyncSurfaces.SESSION, operation: QrSyncOperations.TERMINATE_WITH_ERROR, errorCode: error.code, - phase: from, + phase: phaseFrom, source: QrSyncTelemetrySources.CONTROLLER, + ...(this.state.syncFlow ? { syncFlow: this.state.syncFlow } : {}), }); - this.update((state) => { - state.phase = QrSyncPhases.FAILED; - state.error = error; + this.transitionTo(QrSyncPhases.FAILED, { + errorCode: error.code, + patch: (state) => { + state.error = error; + }, }); return; } diff --git a/app/core/QrSync/constants.ts b/app/core/QrSync/constants.ts index 98b357d79ee..e95a3018c90 100644 --- a/app/core/QrSync/constants.ts +++ b/app/core/QrSync/constants.ts @@ -30,6 +30,15 @@ export const QrSyncPhases = { FAILED: 'failed', } as const; +/** Distinguishes new-user vs existing-user QR sync receive paths. */ +export const QrSyncSyncFlows = { + NEW_USER: 'new_user', + EXISTING_USER: 'existing_user', +} as const; + +export type QrSyncSyncFlow = + (typeof QrSyncSyncFlows)[keyof typeof QrSyncSyncFlows]; + /** * Persisted provisioning pipeline status for QR sync vault import. * diff --git a/app/core/QrSync/controller-types.ts b/app/core/QrSync/controller-types.ts index 8bf9dcb03e7..3556f48c615 100644 --- a/app/core/QrSync/controller-types.ts +++ b/app/core/QrSync/controller-types.ts @@ -15,6 +15,7 @@ import type { QrSyncOtpDisplay, QrSyncPhase, } from './types'; +import type { QrSyncSyncFlow } from './constants'; import { QrSyncProvisioningServiceImportSecretsToVaultAction } from './services/qr-sync-provisioning-service'; /** Runtime IDs written to persisted metadata after vault import (Phase B). */ @@ -29,6 +30,7 @@ export const QR_SYNC_CONTROLLER_NAME = 'QrSyncController'; export type QrSyncControllerState = { phase: QrSyncPhase; connectionStatus: QrSyncConnectionStatus; + syncFlow: QrSyncSyncFlow | null; /** Ephemeral secrets until password import. Never persisted. */ pendingSecretImports: QrSyncSecretImportEntry[] | null; /** Persisted provisioning plan (no secret material). */ diff --git a/app/core/QrSync/qrSyncTelemetry.test.ts b/app/core/QrSync/qrSyncTelemetry.test.ts index d523889b326..428b08907a8 100644 --- a/app/core/QrSync/qrSyncTelemetry.test.ts +++ b/app/core/QrSync/qrSyncTelemetry.test.ts @@ -85,8 +85,8 @@ describe('qrSyncTelemetry', () => { describe('addQrSyncPhaseBreadcrumb', () => { it('emits a phase breadcrumb without secrets', () => { addQrSyncPhaseBreadcrumb({ - from: 'initializing', - to: 'displaying-otp', + phaseFrom: 'initializing', + phaseTo: 'displaying-otp', }); expect(addBreadcrumb).toHaveBeenCalledWith({ @@ -94,16 +94,16 @@ describe('qrSyncTelemetry', () => { level: 'info', message: 'qr_sync.phase initializing->displaying-otp', data: { - from: 'initializing', - to: 'displaying-otp', + phaseFrom: 'initializing', + phaseTo: 'displaying-otp', }, }); }); it('marks failed transitions as error level with errorCode', () => { addQrSyncPhaseBreadcrumb({ - from: 'displaying-otp', - to: 'failed', + phaseFrom: 'displaying-otp', + phaseTo: 'failed', errorCode: 'CHANNEL_DISCONNECTED', }); @@ -113,6 +113,8 @@ describe('qrSyncTelemetry', () => { message: 'qr_sync.phase displaying-otp->failed code=CHANNEL_DISCONNECTED', data: expect.objectContaining({ + phaseFrom: 'displaying-otp', + phaseTo: 'failed', errorCode: 'CHANNEL_DISCONNECTED', }), }), diff --git a/app/core/QrSync/qrSyncTelemetry.ts b/app/core/QrSync/qrSyncTelemetry.ts index c433db502fa..b628a634a1b 100644 --- a/app/core/QrSync/qrSyncTelemetry.ts +++ b/app/core/QrSync/qrSyncTelemetry.ts @@ -8,6 +8,7 @@ import { addBreadcrumb } from '@sentry/react-native'; import Logger, { type LoggerErrorOptions } from '../../util/Logger'; +import { QrSyncSyncFlows, type QrSyncSyncFlow } from './constants'; import type { QrSyncErrorCode, QrSyncPhase } from './types'; /** Sentry tag value for QR sync errors. Query: `feature:qr-sync`. */ @@ -62,14 +63,7 @@ export const QrSyncTelemetrySources = { export type QrSyncTelemetrySource = (typeof QrSyncTelemetrySources)[keyof typeof QrSyncTelemetrySources]; -/** Distinguishes new-user vs existing-user QR sync receive paths. */ -export const QrSyncSyncFlows = { - NEW_USER: 'new_user', - EXISTING_USER: 'existing_user', -} as const; - -export type QrSyncSyncFlow = - (typeof QrSyncSyncFlows)[keyof typeof QrSyncSyncFlows]; +export { QrSyncSyncFlows, type QrSyncSyncFlow }; const SENSITIVE_KEY_PATTERN = /^(otp|mnemonic|seed|seedPhrase|privateKey|pendingSecretImports|value|p|payload|scannedQr|qrPayload|sessionRequest)$/i; @@ -84,16 +78,16 @@ const MNEMONIC_BLOB_PATTERN = /\b(?:[a-z]+(?:\s+[a-z]+){11,23})\b/gi; const REDACTED = '[REDACTED]'; export interface QrSyncPhaseBreadcrumbArgs { - from: QrSyncPhase; - to: QrSyncPhase; - errorCode?: QrSyncErrorCode | string; + phaseFrom: QrSyncPhase; + phaseTo: QrSyncPhase; + errorCode?: QrSyncErrorCode; } export interface ReportQrSyncFailureOptions { surface: QrSyncSurface; operation: QrSyncOperation; - errorCode?: QrSyncErrorCode | string; - phase?: QrSyncPhase | string; + errorCode?: QrSyncErrorCode; + phase?: QrSyncPhase; source?: QrSyncTelemetrySource | string; syncFlow?: QrSyncSyncFlow; /** Extra display-only fields — scrubbed before send. */ @@ -146,25 +140,28 @@ function scrubSensitiveString(raw: string): string { } /** - * Phase-transition breadcrumb without secrets (from/to/errorCode only). + * Phase-transition breadcrumb without secrets (phaseFrom/phaseTo/errorCode only). + * + * Uses `phaseFrom`/`phaseTo` rather than `from`/`to` because Sentry's + * `rewriteBreadcrumb` treats those keys as URLs and drops non-URL values. */ export function addQrSyncPhaseBreadcrumb({ - from, - to, + phaseFrom, + phaseTo, errorCode, }: QrSyncPhaseBreadcrumbArgs): void { - const parts = [`qr_sync.phase ${from}->${to}`]; + const parts = [`qr_sync.phase ${phaseFrom}->${phaseTo}`]; if (errorCode !== undefined) { parts.push(`code=${errorCode}`); } addBreadcrumb({ category: 'qr_sync', - level: to === 'failed' ? 'error' : 'info', + level: phaseTo === 'failed' ? 'error' : 'info', message: parts.join(' '), data: { - from, - to, + phaseFrom, + phaseTo, ...(errorCode !== undefined && { errorCode }), }, }); diff --git a/app/core/QrSync/services/qr-sync-provisioning-service.ts b/app/core/QrSync/services/qr-sync-provisioning-service.ts index d68f36e5989..ccff478307a 100644 --- a/app/core/QrSync/services/qr-sync-provisioning-service.ts +++ b/app/core/QrSync/services/qr-sync-provisioning-service.ts @@ -52,9 +52,9 @@ import { toFormattedAddress } from '../../../util/address'; import { QrSyncOperations, QrSyncSurfaces, - QrSyncSyncFlows, QrSyncTelemetrySources, reportQrSyncFailure, + type QrSyncSyncFlow, } from '../qrSyncTelemetry'; const SERVICE_NAME = 'QrSyncProvisioningService' as const; @@ -153,24 +153,25 @@ export class QrSyncProvisioningService { ); } } else { - const syncFlow = this.#resolveSyncFlow(); + const syncFlow = this.#getSessionSyncFlow(); reportQrSyncFailure( new Error('QrSyncProvisioningService: Unknown secret type'), { surface: QrSyncSurfaces.IMPORT, operation: QrSyncOperations.IMPORT_SECRETS_UNKNOWN_TYPE, source: QrSyncTelemetrySources.PROVISIONING_IMPORT_SECRETS, - syncFlow, + ...(syncFlow ? { syncFlow } : {}), extras: { secretType: String(secret.type) }, }, ); } } catch (error) { + const syncFlow = this.#getSessionSyncFlow(); reportQrSyncFailure(error, { surface: QrSyncSurfaces.IMPORT, operation: QrSyncOperations.IMPORT_SECRETS_TO_VAULT, source: QrSyncTelemetrySources.PROVISIONING_IMPORT_SECRETS, - syncFlow: this.#resolveSyncFlow(), + ...(syncFlow ? { syncFlow } : {}), }); } } @@ -441,33 +442,25 @@ export class QrSyncProvisioningService { try { await this.#messenger.call('AccountTreeController:syncWithUserStorage'); } catch (error) { + const syncFlow = this.#getSessionSyncFlow(); reportQrSyncFailure(error, { surface: QrSyncSurfaces.IMPORT, operation: QrSyncOperations.USER_STORAGE_RECONCILIATION, source: QrSyncTelemetrySources.PROVISIONING_RECONCILE, - syncFlow: this.#resolveSyncFlow(), + ...(syncFlow ? { syncFlow } : {}), }); } } /** - * Best-effort new vs existing-user classification for Sentry tags. - * Pending primary mnemonic usually indicates the new-user onboarding path. + * Returns the sync flow captured on the controller when the QR session started + * (from local onboarding status), not derived from extension payloads. */ - #resolveSyncFlow(): - | typeof QrSyncSyncFlows.NEW_USER - | typeof QrSyncSyncFlows.EXISTING_USER { + #getSessionSyncFlow(): QrSyncSyncFlow | undefined { try { - const { pendingSecretImports } = this.#getQrSyncControllerState(); - const hasPrimaryPendingMnemonic = pendingSecretImports?.some( - (secret) => - secret.type === QrSyncSecretTypes.MNEMONIC && secret.isPrimary, - ); - return hasPrimaryPendingMnemonic - ? QrSyncSyncFlows.NEW_USER - : QrSyncSyncFlows.EXISTING_USER; + return this.#getQrSyncControllerState().syncFlow ?? undefined; } catch { - return QrSyncSyncFlows.EXISTING_USER; + return undefined; } } diff --git a/app/core/QrSync/types.ts b/app/core/QrSync/types.ts index ab95fbcfb04..751738fc451 100644 --- a/app/core/QrSync/types.ts +++ b/app/core/QrSync/types.ts @@ -54,15 +54,19 @@ export interface QrSyncOffer { * - `SYNC_REJECTED` — extension explicitly rejected the sync (peer-originated) * - `SYNC_FAILED` — unexpected runtime or wallet-client failure during an active session */ +export const QrSyncErrorCodes = { + CHANNEL_INIT_FAILED: 'CHANNEL_INIT_FAILED', + CHANNEL_DISCONNECTED: 'CHANNEL_DISCONNECTED', + INVALID_PAYLOAD: 'INVALID_PAYLOAD', + UNSUPPORTED_VERSION: 'UNSUPPORTED_VERSION', + SESSION_EXPIRED: 'SESSION_EXPIRED', + SYNC_REJECTED: 'SYNC_REJECTED', + SYNC_FAILED: 'SYNC_FAILED', + UNKNOWN_ERROR: 'UNKNOWN_ERROR', +} as const; + export type QrSyncErrorCode = - | 'CHANNEL_INIT_FAILED' - | 'CHANNEL_DISCONNECTED' - | 'INVALID_PAYLOAD' - | 'UNSUPPORTED_VERSION' - | 'SESSION_EXPIRED' - | 'SYNC_REJECTED' - | 'SYNC_FAILED' - | 'UNKNOWN_ERROR'; + (typeof QrSyncErrorCodes)[keyof typeof QrSyncErrorCodes]; /** Structured QR sync error surfaced to services and UI bridges. */ // eslint-disable-next-line @typescript-eslint/consistent-type-definitions From 6402cf7e13f7e1bf618eda38aa5e549c842ff161 Mon Sep 17 00:00:00 2001 From: Gaurav Goel Date: Thu, 16 Jul 2026 06:52:46 +0700 Subject: [PATCH 07/10] fix: unit tests --- app/util/test/initial-background-state.json | 1 + 1 file changed, 1 insertion(+) diff --git a/app/util/test/initial-background-state.json b/app/util/test/initial-background-state.json index 795336efa7b..9ea317ea757 100644 --- a/app/util/test/initial-background-state.json +++ b/app/util/test/initial-background-state.json @@ -861,6 +861,7 @@ "QrSyncController": { "phase": "idle", "connectionStatus": "disconnected", + "syncFlow": null, "pendingSecretImports": null, "provisioningMetadata": null, "provisioningStatus": null, From d7c5da005295f77930d7568c706694679ddbd8de Mon Sep 17 00:00:00 2001 From: Gaurav Goel Date: Thu, 16 Jul 2026 11:54:46 +0700 Subject: [PATCH 08/10] fix: sonar warnings --- .../OptinMetrics.navigation.test.tsx | 2 +- .../completeExistingUserQrSyncImport.ts | 2 +- app/core/QrSync/qrSyncTelemetry.ts | 7 +- .../services/qr-sync-provisioning-service.ts | 100 +++++++++++------- app/core/QrSync/useQrSyncImportNavigation.ts | 3 +- .../finalizeOnboardingCompletion.test.ts | 2 +- .../finalizeOnboardingCompletion.ts | 4 +- 7 files changed, 68 insertions(+), 52 deletions(-) diff --git a/app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx b/app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx index b2fd578d5f0..b61ce3889fa 100644 --- a/app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx +++ b/app/components/UI/OptinMetrics/OptinMetrics.navigation.test.tsx @@ -510,7 +510,7 @@ describe('OptinMetrics — interest questionnaire navigation branching', () => { feature: 'qr-sync', surface: 'import', operation: 'provision_from_metadata', - source: 'OptinMetrics', + source: 'finalizeOnboardingCompletion', syncFlow: 'new_user', }), }), diff --git a/app/core/QrSync/completeExistingUserQrSyncImport.ts b/app/core/QrSync/completeExistingUserQrSyncImport.ts index 95841c29b2c..33b8d51e71a 100644 --- a/app/core/QrSync/completeExistingUserQrSyncImport.ts +++ b/app/core/QrSync/completeExistingUserQrSyncImport.ts @@ -5,10 +5,10 @@ import { showImportFailedSheet } from '../../components/Views/AddDeviceToWallet/ import Engine from '../Engine'; import type { AppNavigationProp } from '../NavigationService/types'; import { isDuplicateMnemonicError } from './duplicateMnemonicError'; +import { QrSyncSyncFlows } from './constants'; import { QrSyncOperations, QrSyncSurfaces, - QrSyncSyncFlows, QrSyncTelemetrySources, reportQrSyncFailure, } from './qrSyncTelemetry'; diff --git a/app/core/QrSync/qrSyncTelemetry.ts b/app/core/QrSync/qrSyncTelemetry.ts index b628a634a1b..d64ecb749fa 100644 --- a/app/core/QrSync/qrSyncTelemetry.ts +++ b/app/core/QrSync/qrSyncTelemetry.ts @@ -8,7 +8,7 @@ import { addBreadcrumb } from '@sentry/react-native'; import Logger, { type LoggerErrorOptions } from '../../util/Logger'; -import { QrSyncSyncFlows, type QrSyncSyncFlow } from './constants'; +import type { QrSyncSyncFlow } from './constants'; import type { QrSyncErrorCode, QrSyncPhase } from './types'; /** Sentry tag value for QR sync errors. Query: `feature:qr-sync`. */ @@ -58,13 +58,12 @@ export const QrSyncTelemetrySources = { CONTROLLER_IMPORT_REMAINING: 'QrSyncController.importRemainingSecrets', CONTROLLER_ENRICH_PRIMARY: 'QrSyncController.enrichPrimaryProvisioningEntry', CONTROLLER: 'QrSyncController', + FINALIZE_ONBOARDING: 'finalizeOnboardingCompletion', } as const; export type QrSyncTelemetrySource = (typeof QrSyncTelemetrySources)[keyof typeof QrSyncTelemetrySources]; -export { QrSyncSyncFlows, type QrSyncSyncFlow }; - const SENSITIVE_KEY_PATTERN = /^(otp|mnemonic|seed|seedPhrase|privateKey|pendingSecretImports|value|p|payload|scannedQr|qrPayload|sessionRequest)$/i; @@ -88,7 +87,7 @@ export interface ReportQrSyncFailureOptions { operation: QrSyncOperation; errorCode?: QrSyncErrorCode; phase?: QrSyncPhase; - source?: QrSyncTelemetrySource | string; + source?: QrSyncTelemetrySource; syncFlow?: QrSyncSyncFlow; /** Extra display-only fields — scrubbed before send. */ extras?: Record; diff --git a/app/core/QrSync/services/qr-sync-provisioning-service.ts b/app/core/QrSync/services/qr-sync-provisioning-service.ts index ccff478307a..59d3fc0c955 100644 --- a/app/core/QrSync/services/qr-sync-provisioning-service.ts +++ b/app/core/QrSync/services/qr-sync-provisioning-service.ts @@ -40,7 +40,11 @@ import type { QrSyncControllerGetStateAction, QrSyncControllerMarkProvisioningFailedAction, } from '../controller-types'; -import { QrSyncProvisioningStatuses, QrSyncSecretTypes } from '../constants'; +import { + QrSyncProvisioningStatuses, + QrSyncSecretTypes, + type QrSyncSyncFlow, +} from '../constants'; import type { QrSyncAccountGroup, QrSyncProvisioningMetadata, @@ -54,7 +58,6 @@ import { QrSyncSurfaces, QrSyncTelemetrySources, reportQrSyncFailure, - type QrSyncSyncFlow, } from '../qrSyncTelemetry'; const SERVICE_NAME = 'QrSyncProvisioningService' as const; @@ -133,50 +136,65 @@ export class QrSyncProvisioningService { ): Promise { for (const secret of secrets) { try { - if (secret.type === QrSyncSecretTypes.MNEMONIC) { - const entropySource = await this.#importMnemonicToVault(secret.value); - this.#messenger.call( - 'QrSyncController:enrichProvisioningEntry', - secret.index, - { entropySource }, - ); - } else if (secret.type === QrSyncSecretTypes.PRIVATE_KEY) { - const accountAddress = await this.#importPrivateKeyToVault( - secret.value, - ); - - if (accountAddress) { - this.#messenger.call( - 'QrSyncController:enrichProvisioningEntry', - secret.index, - { accountAddress }, - ); - } - } else { - const syncFlow = this.#getSessionSyncFlow(); - reportQrSyncFailure( - new Error('QrSyncProvisioningService: Unknown secret type'), - { - surface: QrSyncSurfaces.IMPORT, - operation: QrSyncOperations.IMPORT_SECRETS_UNKNOWN_TYPE, - source: QrSyncTelemetrySources.PROVISIONING_IMPORT_SECRETS, - ...(syncFlow ? { syncFlow } : {}), - extras: { secretType: String(secret.type) }, - }, - ); - } + await this.#importSingleSecretToVault(secret); } catch (error) { - const syncFlow = this.#getSessionSyncFlow(); - reportQrSyncFailure(error, { - surface: QrSyncSurfaces.IMPORT, - operation: QrSyncOperations.IMPORT_SECRETS_TO_VAULT, - source: QrSyncTelemetrySources.PROVISIONING_IMPORT_SECRETS, - ...(syncFlow ? { syncFlow } : {}), - }); + this.#reportImportSecretsFailure( + error, + QrSyncOperations.IMPORT_SECRETS_TO_VAULT, + ); } } } + async #importSingleSecretToVault( + secret: QrSyncSecretImportEntry, + ): Promise { + if (secret.type === QrSyncSecretTypes.MNEMONIC) { + const entropySource = await this.#importMnemonicToVault(secret.value); + this.#messenger.call( + 'QrSyncController:enrichProvisioningEntry', + secret.index, + { entropySource }, + ); + return; + } + + if (secret.type === QrSyncSecretTypes.PRIVATE_KEY) { + const accountAddress = await this.#importPrivateKeyToVault(secret.value); + if (accountAddress) { + this.#messenger.call( + 'QrSyncController:enrichProvisioningEntry', + secret.index, + { accountAddress }, + ); + } + return; + } + + this.#reportImportSecretsFailure( + new Error('QrSyncProvisioningService: Unknown secret type'), + QrSyncOperations.IMPORT_SECRETS_UNKNOWN_TYPE, + { secretType: String(secret.type) }, + ); + } + + #reportImportSecretsFailure( + error: unknown, + operation: + | typeof QrSyncOperations.IMPORT_SECRETS_TO_VAULT + | typeof QrSyncOperations.IMPORT_SECRETS_UNKNOWN_TYPE, + extras?: Record, + ): void { + const syncFlow = this.#getSessionSyncFlow(); + reportQrSyncFailure(error, { + surface: QrSyncSurfaces.IMPORT, + operation, + source: QrSyncTelemetrySources.PROVISIONING_IMPORT_SECRETS, + ...(syncFlow ? { syncFlow } : {}), + ...(extras ? { extras } : {}), + }); + } + async #importMnemonicToVault(seed: string): Promise { const mnemonic = mnemonicPhraseToBytes(seed); diff --git a/app/core/QrSync/useQrSyncImportNavigation.ts b/app/core/QrSync/useQrSyncImportNavigation.ts index 58962b78b34..670252f5198 100644 --- a/app/core/QrSync/useQrSyncImportNavigation.ts +++ b/app/core/QrSync/useQrSyncImportNavigation.ts @@ -10,7 +10,7 @@ import { } from '../../selectors/qrSyncController'; import type { AppNavigationProp } from '../NavigationService/types'; import Engine from '../Engine'; -import { QrSyncSecretTypes } from './constants'; +import { QrSyncSecretTypes, QrSyncSyncFlows } from './constants'; import { completeExistingUserQrSyncImport } from './completeExistingUserQrSyncImport'; import { navigateToQrSyncImport } from './navigateToQrSyncImport'; import { showAlreadySyncedSheet } from '../../components/Views/AddDeviceToWallet/showAlreadySyncedSheet'; @@ -19,7 +19,6 @@ import type { QrSyncSecretImportEntry } from './types'; import { QrSyncOperations, QrSyncSurfaces, - QrSyncSyncFlows, QrSyncTelemetrySources, reportQrSyncFailure, } from './qrSyncTelemetry'; diff --git a/app/util/onboarding/finalizeOnboardingCompletion.test.ts b/app/util/onboarding/finalizeOnboardingCompletion.test.ts index c05cf614f9d..16475601bfa 100644 --- a/app/util/onboarding/finalizeOnboardingCompletion.test.ts +++ b/app/util/onboarding/finalizeOnboardingCompletion.test.ts @@ -152,7 +152,7 @@ describe('finalizeOnboardingCompletion', () => { feature: 'qr-sync', surface: 'import', operation: 'provision_from_metadata', - source: 'TestContext', + source: 'finalizeOnboardingCompletion', syncFlow: 'new_user', }), }), diff --git a/app/util/onboarding/finalizeOnboardingCompletion.ts b/app/util/onboarding/finalizeOnboardingCompletion.ts index 574ee801a86..a03dd2ef7eb 100644 --- a/app/util/onboarding/finalizeOnboardingCompletion.ts +++ b/app/util/onboarding/finalizeOnboardingCompletion.ts @@ -10,10 +10,10 @@ import { getOnboardingCompletedAnalyticsPropsFromSuccessFlow } from '../analytic import type { WalletSetupCompletedAttributionAnalyticsPayload } from '../analytics/walletSetupCompletedAttribution'; import { analytics } from '../analytics/analytics'; import Logger from '../Logger'; +import { QrSyncSyncFlows } from '../../core/QrSync/constants'; import { QrSyncOperations, QrSyncSurfaces, - QrSyncSyncFlows, QrSyncTelemetrySources, reportQrSyncFailure, } from '../../core/QrSync/qrSyncTelemetry'; @@ -84,7 +84,7 @@ export function finalizeOnboardingCompletion({ reportQrSyncFailure(error, { surface: QrSyncSurfaces.IMPORT, operation: QrSyncOperations.PROVISION_FROM_METADATA, - source: discoverAccountsLogContext, + source: QrSyncTelemetrySources.FINALIZE_ONBOARDING, syncFlow: QrSyncSyncFlows.NEW_USER, }); }, From 15050e6b91e4420c7da13c032366b6bc6ba4d593 Mon Sep 17 00:00:00 2001 From: Gaurav Goel Date: Thu, 16 Jul 2026 12:32:47 +0700 Subject: [PATCH 09/10] fix: sonar warnings --- app/core/QrSync/qrSyncTelemetry.test.ts | 17 +++++++++++ app/core/QrSync/qrSyncTelemetry.ts | 40 ++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/app/core/QrSync/qrSyncTelemetry.test.ts b/app/core/QrSync/qrSyncTelemetry.test.ts index 428b08907a8..246d929c2c7 100644 --- a/app/core/QrSync/qrSyncTelemetry.test.ts +++ b/app/core/QrSync/qrSyncTelemetry.test.ts @@ -192,5 +192,22 @@ describe('qrSyncTelemetry', () => { }), ); }); + + it('stringifies plain object failures without [object Object]', () => { + reportQrSyncFailure( + { code: 'SYNC_FAILED', detail: 'vault' }, + { + surface: QrSyncSurfaces.SESSION, + operation: QrSyncOperations.TERMINATE_WITH_ERROR, + }, + ); + + expect(Logger.error).toHaveBeenCalledWith( + expect.objectContaining({ + message: '{"code":"SYNC_FAILED","detail":"vault"}', + }), + expect.any(Object), + ); + }); }); }); diff --git a/app/core/QrSync/qrSyncTelemetry.ts b/app/core/QrSync/qrSyncTelemetry.ts index d64ecb749fa..33692e82d5c 100644 --- a/app/core/QrSync/qrSyncTelemetry.ts +++ b/app/core/QrSync/qrSyncTelemetry.ts @@ -127,7 +127,11 @@ export function scrubSensitiveQrSyncData(value: unknown): unknown { return result; } - return String(value); + if (typeof value === 'bigint' || typeof value === 'symbol') { + return value.toString(); + } + + return '[unsupported]'; } function scrubSensitiveString(raw: string): string { @@ -138,6 +142,34 @@ function scrubSensitiveString(raw: string): string { .replace(MNEMONIC_BLOB_PATTERN, REDACTED); } +/** + * Convert unknown failure values into a safe display string without + * falling back to Object's default `[object Object]` stringification. + */ +function toSafeDisplayString(value: unknown): string { + if (value instanceof Error) { + return value.message; + } + if (typeof value === 'string') { + return value; + } + if ( + typeof value === 'number' || + typeof value === 'boolean' || + typeof value === 'bigint' + ) { + return value.toString(); + } + if (value === null || value === undefined) { + return ''; + } + try { + return JSON.stringify(value) ?? ''; + } catch { + return Object.prototype.toString.call(value); + } +} + /** * Phase-transition breadcrumb without secrets (phaseFrom/phaseTo/errorCode only). * @@ -179,8 +211,7 @@ export function buildQrSyncLoggerErrorOptions({ syncFlow, extras, }: ReportQrSyncFailureOptions & { error: unknown }): LoggerErrorOptions { - const errorMessage = - error instanceof Error ? error.message : String(error ?? ''); + const errorMessage = toSafeDisplayString(error); const scrubbedExtras = scrubSensitiveQrSyncData({ message: `qr-sync ${surface}.${operation}`, @@ -224,7 +255,8 @@ export function reportQrSyncFailure( error: unknown, options: ReportQrSyncFailureOptions, ): void { - const err = error instanceof Error ? error : new Error(String(error ?? '')); + const err = + error instanceof Error ? error : new Error(toSafeDisplayString(error)); Logger.error( err, buildQrSyncLoggerErrorOptions({ From cb16eaaee5f57688d924a8acbd0da1b6aa68d896 Mon Sep 17 00:00:00 2001 From: Gaurav Goel Date: Thu, 16 Jul 2026 13:10:27 +0700 Subject: [PATCH 10/10] increase code coverage --- .../Views/AddDeviceToWallet/index.test.tsx | 74 ++++++++++ app/core/QrSync/QrSyncController.test.ts | 105 ++++++++++++++ app/core/QrSync/qrSyncTelemetry.test.ts | 88 ++++++++++++ .../qr-sync-provisioning-service.test.ts | 132 +++++++++++++++++- 4 files changed, 395 insertions(+), 4 deletions(-) diff --git a/app/components/Views/AddDeviceToWallet/index.test.tsx b/app/components/Views/AddDeviceToWallet/index.test.tsx index 054eae58e33..ffdec57b1a8 100644 --- a/app/components/Views/AddDeviceToWallet/index.test.tsx +++ b/app/components/Views/AddDeviceToWallet/index.test.tsx @@ -11,14 +11,26 @@ import { import { defaultQrSyncControllerState } from '../../../core/QrSync/QrSyncController'; import AddDeviceToWallet from './index'; import { completeExistingUserQrSyncImport } from '../../../core/QrSync/completeExistingUserQrSyncImport'; +import { + QrSyncOperations, + QrSyncSurfaces, + QrSyncTelemetrySources, + reportQrSyncFailure, +} from '../../../core/QrSync/qrSyncTelemetry'; jest.mock('../../../core/QrSync/completeExistingUserQrSyncImport', () => ({ completeExistingUserQrSyncImport: jest.fn(() => Promise.resolve()), })); +jest.mock('../../../core/QrSync/qrSyncTelemetry', () => ({ + ...jest.requireActual('../../../core/QrSync/qrSyncTelemetry'), + reportQrSyncFailure: jest.fn(), +})); + const mockCompleteExistingUserQrSyncImport = jest.mocked( completeExistingUserQrSyncImport, ); +const mockReportQrSyncFailure = jest.mocked(reportQrSyncFailure); jest.mock('@metamask/design-system-twrnc-preset', () => ({ useTailwind: () => ({ @@ -48,6 +60,8 @@ import Engine from '../../../core/Engine'; const mockCancelSession = Engine.context.QrSyncController .cancelSession as jest.Mock; const mockResetState = Engine.context.QrSyncController.resetState as jest.Mock; +const mockHandleScannedQrPayload = Engine.context.QrSyncController + .handleScannedQrPayload as jest.Mock; const mockNavigate = jest.fn(); const mockGoBack = jest.fn(); @@ -227,6 +241,66 @@ describe('AddDeviceToWallet', () => { }), ); }); + + it('reports scan submit failures to Sentry', async () => { + mockHandleScannedQrPayload.mockRejectedValueOnce( + new Error('scan submit failed'), + ); + const { getByText } = renderComponent(); + + fireEvent.press( + getByText(strings('app_settings.add_device.scan_qr_code_button')), + ); + + const onScanSuccess = mockNavigate.mock.calls[0][1].onScanSuccess as ( + data: { content?: string }, + content?: string, + ) => void; + onScanSuccess({ content: 'metamask://connect/mwp?p=test' }); + + await waitFor(() => { + expect(mockReportQrSyncFailure).toHaveBeenCalledWith( + expect.any(Error), + { + surface: QrSyncSurfaces.SCANNER, + operation: QrSyncOperations.SUBMIT_SCANNED_PAYLOAD, + source: QrSyncTelemetrySources.ADD_DEVICE_ON_SCAN_SUCCESS, + }, + ); + }); + }); + + it('reports manual QR submit failures to Sentry in dev', async () => { + const globalWithDev = global as unknown as { __DEV__: boolean }; + const originalDev = globalWithDev.__DEV__; + globalWithDev.__DEV__ = true; + mockHandleScannedQrPayload.mockRejectedValueOnce( + new Error('manual submit failed'), + ); + + try { + const { getByPlaceholderText, getByText } = renderComponent(); + + fireEvent.changeText( + getByPlaceholderText('Paste QR payload'), + 'metamask://connect/mwp?p=manual', + ); + fireEvent.press(getByText('Submit QR data')); + + await waitFor(() => { + expect(mockReportQrSyncFailure).toHaveBeenCalledWith( + expect.any(Error), + { + surface: QrSyncSurfaces.SCANNER, + operation: QrSyncOperations.SUBMIT_MANUAL_PAYLOAD, + source: QrSyncTelemetrySources.ADD_DEVICE_MANUAL_SUBMIT, + }, + ); + }); + } finally { + globalWithDev.__DEV__ = originalDev; + } + }); }); describe('QR sync presentation', () => { diff --git a/app/core/QrSync/QrSyncController.test.ts b/app/core/QrSync/QrSyncController.test.ts index 0013fa0ac3d..45de7f0f333 100644 --- a/app/core/QrSync/QrSyncController.test.ts +++ b/app/core/QrSync/QrSyncController.test.ts @@ -25,8 +25,20 @@ import { import { createQrSyncWalletClient } from './services/create-qr-sync-wallet-client'; import { QR_SYNC_MWP_DEEPLINK_PREFIX } from './services/qr-sync-validation'; import type { QrSyncSyncReadyMessage } from './types'; +import { + QrSyncOperations, + QrSyncSurfaces, + QrSyncTelemetrySources, + reportQrSyncFailure, +} from './qrSyncTelemetry'; jest.mock('./services/create-qr-sync-wallet-client'); +jest.mock('./qrSyncTelemetry', () => ({ + ...jest.requireActual('./qrSyncTelemetry'), + reportQrSyncFailure: jest.fn(), +})); + +const mockReportQrSyncFailure = jest.mocked(reportQrSyncFailure); const mockCreateQrSyncWalletClient = createQrSyncWalletClient as jest.MockedFunction< @@ -243,6 +255,23 @@ describe('QrSyncController', () => { expect(controller.state.syncFlow).toBe(QrSyncSyncFlows.EXISTING_USER); }); + it('captures NEW_USER syncFlow when onboarding is incomplete', async () => { + const controller = buildController({ + getIsOnboardingCompleted: () => false, + }); + const walletClient = buildMockWalletClient(); + + mockCreateQrSyncWalletClient.mockResolvedValue({ + sessionId: VALID_SESSION_ID, + client: walletClient.client, + }); + + await controller.handleScannedQrPayload(buildValidScanPayload()); + await flushPromises(); + + expect(controller.state.syncFlow).toBe(QrSyncSyncFlows.NEW_USER); + }); + it('throws when the scan payload cannot be parsed', async () => { const controller = buildController(); @@ -691,5 +720,81 @@ describe('QrSyncController', () => { expect(callSpy).not.toHaveBeenCalled(); }); + + it('reports finalize failures when provisioning metadata is missing', async () => { + const messenger = buildMessenger(); + jest.spyOn(messenger, 'call').mockResolvedValue(undefined); + + const orchestratingController = new QrSyncController({ + messenger, + keyManager: {} as IKeyManager, + relayUrl: TEST_RELAY_URL, + getIsOnboardingCompleted: () => false, + }); + const walletClient = buildMockWalletClient(); + + await startSession(orchestratingController, walletClient); + walletClient.emit('message', createSyncReadyWireMessage()); + await flushPromises(); + + ( + orchestratingController as unknown as { + update: ( + fn: (state: typeof orchestratingController.state) => void, + ) => void; + } + ).update((state) => { + state.provisioningMetadata = null; + }); + + await orchestratingController.importRemainingSecrets(); + + expect(mockReportQrSyncFailure).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'QR sync finalize requires provisioning metadata', + }), + expect.objectContaining({ + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.IMPORT_REMAINING_SECRETS_FINALIZE, + source: QrSyncTelemetrySources.CONTROLLER_IMPORT_REMAINING, + }), + ); + }); + + it('reports enrich-primary failures without throwing', async () => { + const messenger = buildMessenger(); + const orchestratingController = new QrSyncController({ + messenger, + keyManager: {} as IKeyManager, + relayUrl: TEST_RELAY_URL, + getIsOnboardingCompleted: () => false, + }); + const walletClient = buildMockWalletClient(); + + await startSession(orchestratingController, walletClient); + walletClient.emit('message', createSyncReadyWireMessage()); + await flushPromises(); + + jest + .spyOn(orchestratingController, 'enrichProvisioningEntry') + .mockImplementation(() => { + throw new Error('enrich failed'); + }); + + expect(() => + orchestratingController.enrichPrimaryProvisioningEntry( + 'primary-entropy' as EntropySourceId, + ), + ).not.toThrow(); + + expect(mockReportQrSyncFailure).toHaveBeenCalledWith( + expect.any(Error), + expect.objectContaining({ + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.ENRICH_PRIMARY_PROVISIONING_ENTRY, + source: QrSyncTelemetrySources.CONTROLLER_ENRICH_PRIMARY, + }), + ); + }); }); }); diff --git a/app/core/QrSync/qrSyncTelemetry.test.ts b/app/core/QrSync/qrSyncTelemetry.test.ts index 246d929c2c7..d09fe1a04ea 100644 --- a/app/core/QrSync/qrSyncTelemetry.test.ts +++ b/app/core/QrSync/qrSyncTelemetry.test.ts @@ -80,6 +80,37 @@ describe('qrSyncTelemetry', () => { expect(scrubbed.extras.note).toBe('ok'); expect(scrubbed.extras.content).toBe('[REDACTED]'); }); + + it('preserves null, undefined, numbers, booleans, and arrays', () => { + expect(scrubSensitiveQrSyncData(null)).toBeNull(); + expect(scrubSensitiveQrSyncData(undefined)).toBeUndefined(); + expect(scrubSensitiveQrSyncData(42)).toBe(42); + expect(scrubSensitiveQrSyncData(true)).toBe(true); + expect(scrubSensitiveQrSyncData([TEST_ADDRESS, 'safe'])).toEqual([ + '[REDACTED]', + 'safe', + ]); + }); + + it('stringifies bigint and symbol values', () => { + expect(scrubSensitiveQrSyncData(10n)).toBe('10'); + expect(scrubSensitiveQrSyncData(Symbol.for('qr-sync'))).toBe( + 'Symbol(qr-sync)', + ); + }); + + it('redacts e2e qr-sync deeplinks', () => { + const scrubbed = scrubSensitiveQrSyncData( + 'open metamask://e2e/qr-sync/payload?token=abc and e2e://qr-sync/x', + ) as string; + + expect(scrubbed).not.toContain('e2e/qr-sync'); + expect(scrubbed).toContain('[REDACTED]'); + }); + + it('marks unsupported value types', () => { + expect(scrubSensitiveQrSyncData(() => undefined)).toBe('[unsupported]'); + }); }); describe('addQrSyncPhaseBreadcrumb', () => { @@ -209,5 +240,62 @@ describe('qrSyncTelemetry', () => { expect.any(Object), ); }); + + it('stringifies primitive non-Error failures', () => { + reportQrSyncFailure(404, { + surface: QrSyncSurfaces.SESSION, + operation: QrSyncOperations.TERMINATE_WITH_ERROR, + }); + reportQrSyncFailure(true, { + surface: QrSyncSurfaces.SESSION, + operation: QrSyncOperations.TERMINATE_WITH_ERROR, + }); + reportQrSyncFailure(7n, { + surface: QrSyncSurfaces.SESSION, + operation: QrSyncOperations.TERMINATE_WITH_ERROR, + }); + reportQrSyncFailure(null, { + surface: QrSyncSurfaces.SESSION, + operation: QrSyncOperations.TERMINATE_WITH_ERROR, + }); + + expect(Logger.error).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ message: '404' }), + expect.any(Object), + ); + expect(Logger.error).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ message: 'true' }), + expect.any(Object), + ); + expect(Logger.error).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ message: '7' }), + expect.any(Object), + ); + expect(Logger.error).toHaveBeenNthCalledWith( + 4, + expect.objectContaining({ message: '' }), + expect.any(Object), + ); + }); + + it('falls back when JSON.stringify cannot serialize the failure', () => { + const circular: Record = {}; + circular.self = circular; + + reportQrSyncFailure(circular, { + surface: QrSyncSurfaces.SESSION, + operation: QrSyncOperations.TERMINATE_WITH_ERROR, + }); + + expect(Logger.error).toHaveBeenCalledWith( + expect.objectContaining({ + message: '[object Object]', + }), + expect.any(Object), + ); + }); }); }); diff --git a/app/core/QrSync/services/qr-sync-provisioning-service.test.ts b/app/core/QrSync/services/qr-sync-provisioning-service.test.ts index a6bb3c449c4..9420a307abd 100644 --- a/app/core/QrSync/services/qr-sync-provisioning-service.test.ts +++ b/app/core/QrSync/services/qr-sync-provisioning-service.test.ts @@ -14,20 +14,34 @@ import { QrSyncMessageVersion, QrSyncProvisioningStatuses, QrSyncSecretTypes, + QrSyncSyncFlows, } from '../constants'; import { defaultQrSyncControllerState } from '../QrSyncController'; import type { QrSyncControllerState } from '../controller-types'; import type { QrSyncProvisioningMetadata } from '../types'; +import { + QrSyncOperations, + QrSyncSurfaces, + QrSyncTelemetrySources, + reportQrSyncFailure, +} from '../qrSyncTelemetry'; jest.mock('@metamask/key-tree', () => ({ mnemonicPhraseToBytes: jest.fn().mockReturnValue(new Uint8Array([1, 2, 3])), })); +jest.mock('../qrSyncTelemetry', () => ({ + ...jest.requireActual('../qrSyncTelemetry'), + reportQrSyncFailure: jest.fn(), +})); + import { QrSyncProvisioningService, type QrSyncProvisioningServiceMessenger, } from './qr-sync-provisioning-service'; +const mockReportQrSyncFailure = jest.mocked(reportQrSyncFailure); + const PRIMARY_ENTROPY_SOURCE = 'primary-entropy-source' as EntropySourceId; const SECONDARY_ENTROPY_SOURCE = 'secondary-entropy-source' as EntropySourceId; const PRIMARY_ADDRESS = '0x1111111111111111111111111111111111111111'; @@ -189,9 +203,6 @@ describe('QrSyncProvisioningService', () => { }); describe('importSecretsToVault', () => { - const SECONDARY_ENTROPY_SOURCE = - 'secondary-entropy-source' as EntropySourceId; - const remainingSecrets = [ { index: 1, @@ -258,6 +269,108 @@ describe('QrSyncProvisioningService', () => { { accountAddress: PRIVATE_KEY_ADDRESS }, ); }); + + it('reports unknown secret types with session syncFlow', async () => { + mockMessenger.call = jest.fn((action: string) => { + if (action === 'QrSyncController:getState') { + return createSecretsImportedState({ + syncFlow: QrSyncSyncFlows.EXISTING_USER, + }); + } + + return undefined; + }); + const importService = new QrSyncProvisioningService({ + messenger: asProvisioningMessenger(mockMessenger), + }); + + await importService.importSecretsToVault([ + { + index: 0, + type: 'unknown' as typeof QrSyncSecretTypes.MNEMONIC, + value: 'ignored', + }, + ]); + + expect(mockReportQrSyncFailure).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'QrSyncProvisioningService: Unknown secret type', + }), + expect.objectContaining({ + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.IMPORT_SECRETS_UNKNOWN_TYPE, + source: QrSyncTelemetrySources.PROVISIONING_IMPORT_SECRETS, + syncFlow: QrSyncSyncFlows.EXISTING_USER, + extras: { secretType: 'unknown' }, + }), + ); + }); + + it('reports vault import failures without syncFlow when state lookup fails', async () => { + mockMessenger.call = jest.fn((action: string) => { + if ( + action === 'MultichainAccountService:createMultichainAccountWallet' + ) { + return Promise.reject(new Error('import mnemonic failed')); + } + + if (action === 'QrSyncController:getState') { + throw new Error('state unavailable'); + } + + return undefined; + }); + const importService = new QrSyncProvisioningService({ + messenger: asProvisioningMessenger(mockMessenger), + }); + + await importService.importSecretsToVault([ + { + index: 1, + type: QrSyncSecretTypes.MNEMONIC, + value: 'secondary mnemonic', + }, + ]); + + expect(mockReportQrSyncFailure).toHaveBeenCalledWith( + expect.any(Error), + expect.objectContaining({ + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.IMPORT_SECRETS_TO_VAULT, + source: QrSyncTelemetrySources.PROVISIONING_IMPORT_SECRETS, + }), + ); + expect(mockReportQrSyncFailure.mock.calls[0][1]).not.toHaveProperty( + 'syncFlow', + ); + }); + + it('skips private-key enrichment when import returns an empty address', async () => { + mockMessenger.call = jest.fn((action: string) => { + if (action === 'KeyringController:importAccountWithStrategy') { + return Promise.resolve(''); + } + + return undefined; + }); + const importService = new QrSyncProvisioningService({ + messenger: asProvisioningMessenger(mockMessenger), + }); + + await importService.importSecretsToVault([ + { + index: 2, + type: QrSyncSecretTypes.PRIVATE_KEY, + value: '0xabc', + }, + ]); + + expect(mockMessenger.call).not.toHaveBeenCalledWith( + 'QrSyncController:enrichProvisioningEntry', + expect.anything(), + expect.anything(), + ); + }); }); describe('provisionFromMetadata group creation', () => { @@ -451,7 +564,9 @@ describe('QrSyncProvisioningService', () => { it('completes provisioning when user storage reconciliation fails', async () => { const mockCall = jest.fn((action: string, ...args: unknown[]) => { if (action === 'QrSyncController:getState') { - return createSecretsImportedState(); + return createSecretsImportedState({ + syncFlow: QrSyncSyncFlows.NEW_USER, + }); } if (action === 'AccountTreeController:getAccountWalletObjects') { @@ -487,6 +602,15 @@ describe('QrSyncProvisioningService', () => { expect(mockCall).not.toHaveBeenCalledWith( 'QrSyncController:markProvisioningFailed', ); + expect(mockReportQrSyncFailure).toHaveBeenCalledWith( + expect.any(Error), + expect.objectContaining({ + surface: QrSyncSurfaces.IMPORT, + operation: QrSyncOperations.USER_STORAGE_RECONCILIATION, + source: QrSyncTelemetrySources.PROVISIONING_RECONCILE, + syncFlow: QrSyncSyncFlows.NEW_USER, + }), + ); }); it('marks provisioning failed and rethrows when metadata provisioning fails', async () => {