Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions app/components/Views/AddDeviceToWallet/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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',
Comment thread
grvgoel81 marked this conversation as resolved.
Outdated
operation: 'submit_scanned_payload',
source: 'AddDeviceToWallet.onScanSuccess',
});
});
},
[submitQrPayload],
Expand Down Expand Up @@ -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]);

Expand Down
13 changes: 13 additions & 0 deletions app/components/Views/QRScanner/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>((resolve) => {
Expand Down Expand Up @@ -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'
Comment thread
grvgoel81 marked this conversation as resolved.
Outdated
? 'SESSION_EXPIRED'
: 'INVALID_PAYLOAD',
source: 'QRScanner.addDevice',
},
);
trackEvent(
createEventBuilder(MetaMetricsEvents.QR_SCANNED)
.addProperties({
Expand Down
32 changes: 32 additions & 0 deletions app/core/QrSync/QrSyncController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
116 changes: 106 additions & 10 deletions app/core/QrSync/QrSyncController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<QrSyncControllerState> = {
phase: {
Expand Down Expand Up @@ -112,6 +115,8 @@ export class QrSyncController extends BaseController<

private sessionId: string | null = null;

private grantWaitTimeoutId: ReturnType<typeof setTimeout> | null = null;

constructor({
messenger,
state,
Expand Down Expand Up @@ -221,10 +226,12 @@ export class QrSyncController extends BaseController<
try {
this.finalizeSecretImport();
} catch (error) {
Logger.error(
Comment thread
grvgoel81 marked this conversation as resolved.
error as Error,
'QrSyncController.importRemainingSecrets finalize',
);
reportQrSyncFailure(error, {
surface: 'import',
operation: 'import_remaining_secrets_finalize',
phase: this.state.phase,
source: 'QrSyncController.importRemainingSecrets',
});
}
}

Expand Down Expand Up @@ -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();
Comment thread
grvgoel81 marked this conversation as resolved.
Outdated
this.setConnectionStatus('connected');
};

Expand Down Expand Up @@ -390,21 +398,45 @@ export class QrSyncController extends BaseController<
private readonly handleSessionServiceEvent = (event: QrSyncServiceEvent) => {
switch (event.type) {
case QrSyncActionTypes.OTP_DISPLAY_GRANT: {
const from = this.state.phase;
Comment thread
grvgoel81 marked this conversation as resolved.
Outdated
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);
Comment thread
grvgoel81 marked this conversation as resolved.
Outdated
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;
});
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;
Expand All @@ -414,6 +446,7 @@ export class QrSyncController extends BaseController<
break;
}
case QrSyncActionTypes.SYNC_CANCEL: {
this.clearGrantWaitTimeout();
this.clearControllerState();
this.destroySession().catch(() => undefined);
break;
Expand All @@ -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<void> {
Expand All @@ -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();
}

Expand Down Expand Up @@ -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',
});
}
}

Expand Down Expand Up @@ -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;
});
Expand All @@ -529,12 +583,36 @@ export class QrSyncController extends BaseController<
);
}

private scheduleGrantWaitTimeout(deadline: number): void {
Comment thread
grvgoel81 marked this conversation as resolved.
Outdated
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<void> {
this.clearGrantWaitTimeout();

if (this.client) {
try {
if (wireType === QrSyncActionTypes.SYNC_ERROR && error) {
Expand All @@ -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;
Expand All @@ -568,6 +661,8 @@ export class QrSyncController extends BaseController<
}

private async destroySession(): Promise<void> {
this.clearGrantWaitTimeout();

if (!this.client) {
return;
}
Expand Down Expand Up @@ -616,6 +711,7 @@ export class QrSyncController extends BaseController<
}

private clearControllerState(): void {
this.clearGrantWaitTimeout();
this.update(() => ({
...defaultQrSyncControllerState,
}));
Expand Down
6 changes: 6 additions & 0 deletions app/core/QrSync/completeExistingUserQrSyncImport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
Expand Down
Loading
Loading