Skip to content

Commit 5dd99a1

Browse files
authored
feat: add new hardware wallet signing qr code (#29737)
## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> Wires the QR scan errors from PR 1 into pairing and signing flows. QR scan failures now surface through the hardware wallet bottom sheet with retry support, signing confirmations can reopen the scanner from the error CTA, and replacement transaction gas params are normalized through the shared helper. This is PR 2 of 3 and is stacked on #29388. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Improved retry behavior when QR hardware wallet signing scans fail ## **Related issues** Refs: MUL-1665 ## **Manual testing steps** ```gherkin Feature: QR hardware signing retry Scenario: user retries a failed QR signing scan Given the user is signing with a QR-based hardware wallet When the user scans an invalid QR code Then the hardware wallet bottom sheet displays the QR scan error When the user taps Try again Then the QR scanner reopens for another signing scan ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** N/A ### **After** N/A ## **Pre-merge author checklist** <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. #### Performance checks (if applicable) - [x] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [x] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [x] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. Made with [Cursor](https://cursor.com) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Moderate risk because it changes the hardware-wallet bottom-sheet recovery flow and retry behavior for QR signing, which could affect signing UX/state transitions. Adds new analytics properties for QR scan failures; incorrect classification could skew metrics but not funds. > > **Overview** > **Improves QR hardware wallet scan-failure recovery during pairing/signing.** QR scan errors now route through the hardware wallet bottom sheet with a dedicated retry path that can reopen the QR scanner for signing retries, plus a provider-level `setQrScanRetryHandler`/`onRetryQrScan` mechanism to coordinate retries outside provider-managed flows. > > **Adds QR scan error-specific UI + analytics.** `ErrorContent` treats QR scan errors specially (custom title, *Try again* + *Learn more*, no generic icon) and supports `OPEN_SETTINGS` recovery; `useHardwareWalletAnalytics` now attaches QR scan metadata (`error_category`, `is_ur_format`, optional `received_ur_type`) to recovery viewed/CTA/success events. > > Includes supporting refactors and tests (QR adapter no longer emits `AppOpened` on `ensureDeviceReady`, new `isQRHardwareScanError`, `useQrScanErrorForwarding`, `useIsConfirmationFromQrAccount`, and `useQrConfirm`). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 31443d4. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 38817db commit 5dd99a1

25 files changed

Lines changed: 1487 additions & 99 deletions

app/core/HardwareWallet/HardwareWalletProvider.tsx

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ export const HardwareWalletProvider: React.FC<HardwareWalletProviderProps> = ({
8888

8989
const awaitingConfirmationRejectRef = useRef<(() => void) | null>(null);
9090
const operationTypeRef = useRef<'transaction' | 'message' | null>(null);
91+
const qrScanRetryHandlerRef = useRef<(() => void) | null>(null);
9192

9293
const [analyticsFlow, setAnalyticsFlow] = useState(
9394
HardwareWalletAnalyticsFlow.Connection,
@@ -136,6 +137,10 @@ export const HardwareWalletProvider: React.FC<HardwareWalletProviderProps> = ({
136137
[handleError],
137138
);
138139

140+
const setQrScanRetryHandler = useCallback((handler: (() => void) | null) => {
141+
qrScanRetryHandlerRef.current = handler;
142+
}, []);
143+
139144
const showAwaitingConfirmation = useCallback(
140145
(operationType: 'transaction' | 'message', onReject?: () => void) => {
141146
DevLogger.log(
@@ -187,6 +192,26 @@ export const HardwareWalletProvider: React.FC<HardwareWalletProviderProps> = ({
187192
}
188193
await retryEnsureDeviceReady();
189194
}, [handleCloseFlow, retryEnsureDeviceReady]);
195+
196+
const handleRetryQrScan = useCallback(() => {
197+
if (operationTypeRef.current !== null) {
198+
updateConnectionState({
199+
status: ConnectionStatus.AwaitingConfirmation,
200+
deviceId: deviceId ?? 'unknown',
201+
operationType: operationTypeRef.current,
202+
});
203+
return;
204+
}
205+
206+
const retryQrScan = qrScanRetryHandlerRef.current;
207+
updateConnectionState({ status: ConnectionStatus.Disconnected });
208+
if (!retryQrScan) {
209+
return;
210+
}
211+
212+
retryQrScan();
213+
}, [deviceId, updateConnectionState]);
214+
190215
const handleAwaitingConfirmationCancel = useCallback(() => {
191216
DevLogger.log('[HardwareWallet] handleAwaitingConfirmationCancel');
192217
const onReject = awaitingConfirmationRejectRef.current;
@@ -248,6 +273,7 @@ export const HardwareWalletProvider: React.FC<HardwareWalletProviderProps> = ({
248273
setTargetWalletType: setters.setTargetWalletType,
249274
setPendingOperationAddress,
250275
showHardwareWalletError,
276+
setQrScanRetryHandler,
251277
showAwaitingConfirmation,
252278
hideAwaitingConfirmation,
253279
qr: qrSigningValue,
@@ -261,6 +287,7 @@ export const HardwareWalletProvider: React.FC<HardwareWalletProviderProps> = ({
261287
setters.setTargetWalletType,
262288
setPendingOperationAddress,
263289
showHardwareWalletError,
290+
setQrScanRetryHandler,
264291
showAwaitingConfirmation,
265292
hideAwaitingConfirmation,
266293
qrSigningValue,
@@ -282,6 +309,7 @@ export const HardwareWalletProvider: React.FC<HardwareWalletProviderProps> = ({
282309
onAwaitingConfirmationCancel={handleAwaitingConfirmationCancel}
283310
onConnectionSuccess={handleBottomSheetConnectionSuccess}
284311
onCTAClicked={trackCTAClicked}
312+
onRetryQrScan={handleRetryQrScan}
285313
/>
286314
</HardwareWalletContext.Provider>
287315
);

app/core/HardwareWallet/adapters/QRWalletAdapter.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,11 @@ describe('QRWalletAdapter', () => {
128128
).rejects.toThrow('Adapter has been destroyed');
129129
});
130130

131-
it('returns true and emits AppOpened (QR wallets are always ready)', async () => {
131+
it('returns true when camera permission is granted (QR wallets are always ready)', async () => {
132132
const result = await adapter.ensureDeviceReady('qr-account-address');
133133

134134
expect(result).toBe(true);
135-
expect(onDeviceEvent).toHaveBeenCalledWith({
136-
event: DeviceEvent.AppOpened,
137-
});
135+
expect(onDeviceEvent).not.toHaveBeenCalled();
138136
});
139137

140138
it('stores device ID', async () => {
@@ -150,6 +148,7 @@ describe('QRWalletAdapter', () => {
150148
const result = await adapter.ensureDeviceReady('qr-account-address');
151149

152150
expect(result).toBe(false);
151+
expect(mockRequestCameraPermission).not.toHaveBeenCalled();
153152
expect(adapter.getConnectedDeviceId()).toBeNull();
154153
expect(adapter.isConnected()).toBe(false);
155154
expect(onDeviceEvent).toHaveBeenCalledWith({

app/core/HardwareWallet/adapters/QRWalletAdapter.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,6 @@ export class QRWalletAdapter implements HardwareWalletAdapter {
114114

115115
DevLogger.log('[QRWalletAdapter] Device is ready');
116116

117-
// For QR wallets, we consider the "app" to always be open
118-
// since there's no app concept like on Ledger
119-
this.#emitEvent({
120-
event: DeviceEvent.AppOpened,
121-
});
122-
123117
return true;
124118
}
125119

@@ -271,12 +265,8 @@ export class QRWalletAdapter implements HardwareWalletAdapter {
271265
if (newStatus === 'granted') {
272266
return true;
273267
}
274-
275-
this.#emitCameraPermissionDenied();
276-
return false;
277268
}
278269

279-
// status === 'denied' - emit error event
280270
this.#emitCameraPermissionDenied();
281271
return false;
282272
} catch (error) {

app/core/HardwareWallet/analytics/helpers.test.ts

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ import {
1616
getAnalyticsDeviceType,
1717
getErrorDetails,
1818
getAnalyticsFlowFromApproval,
19+
getQrHardwareScanErrorAnalyticsProperties,
1920
} from './helpers';
21+
import { createQRHardwareScanError, QRHardwareScanErrorType } from '../errors';
22+
import { QrScanRequestType } from '@metamask/eth-qr-keyring';
2023

2124
describe('analytics helpers', () => {
2225
describe('getAnalyticsErrorType', () => {
@@ -362,4 +365,105 @@ describe('analytics helpers', () => {
362365
);
363366
});
364367
});
368+
369+
describe('getQrHardwareScanErrorAnalyticsProperties', () => {
370+
it('returns empty object for non-ErrorState', () => {
371+
const state: HardwareWalletConnectionState = {
372+
status: ConnectionStatus.Disconnected,
373+
};
374+
375+
expect(getQrHardwareScanErrorAnalyticsProperties(state)).toEqual({});
376+
});
377+
378+
it('returns empty object for ErrorState with non-QR error', () => {
379+
const error = new HardwareWalletError('Test', {
380+
code: ErrorCode.Unknown,
381+
severity: Severity.Err,
382+
category: Category.Connection,
383+
userMessage: 'Test',
384+
});
385+
386+
const state: HardwareWalletConnectionState = {
387+
status: ConnectionStatus.ErrorState,
388+
error,
389+
};
390+
391+
expect(getQrHardwareScanErrorAnalyticsProperties(state)).toEqual({});
392+
});
393+
394+
it('returns QR scan properties for non-UR QR scanned error', () => {
395+
const qrError = createQRHardwareScanError({
396+
errorType: QRHardwareScanErrorType.NonURQrScanned,
397+
purpose: QrScanRequestType.SIGN,
398+
isUrFormat: false,
399+
});
400+
401+
const state: HardwareWalletConnectionState = {
402+
status: ConnectionStatus.ErrorState,
403+
error: qrError,
404+
};
405+
406+
const result = getQrHardwareScanErrorAnalyticsProperties(state);
407+
expect(result).toEqual({
408+
error_category: 'non_ur_qr_scanned',
409+
is_ur_format: false,
410+
});
411+
});
412+
413+
it('returns received_ur_type for wrong UR type error', () => {
414+
const qrError = createQRHardwareScanError({
415+
errorType: QRHardwareScanErrorType.WrongURType,
416+
purpose: QrScanRequestType.SIGN,
417+
receivedUrType: 'crypto-account',
418+
isUrFormat: true,
419+
});
420+
421+
const state: HardwareWalletConnectionState = {
422+
status: ConnectionStatus.ErrorState,
423+
error: qrError,
424+
};
425+
426+
const result = getQrHardwareScanErrorAnalyticsProperties(state);
427+
expect(result).toEqual({
428+
error_category: 'wrong_ur_type',
429+
is_ur_format: true,
430+
received_ur_type: 'crypto-account',
431+
});
432+
});
433+
434+
it('returns empty string for received_ur_type when not provided', () => {
435+
const qrError = createQRHardwareScanError({
436+
errorType: QRHardwareScanErrorType.WrongURType,
437+
purpose: QrScanRequestType.PAIR,
438+
isUrFormat: true,
439+
});
440+
441+
const state: HardwareWalletConnectionState = {
442+
status: ConnectionStatus.ErrorState,
443+
error: qrError,
444+
};
445+
446+
const result = getQrHardwareScanErrorAnalyticsProperties(state);
447+
expect(result.received_ur_type).toBe('');
448+
});
449+
450+
it('returns QR scan properties for UR decode error', () => {
451+
const qrError = createQRHardwareScanError({
452+
errorType: QRHardwareScanErrorType.URDecodeError,
453+
purpose: QrScanRequestType.SIGN,
454+
isUrFormat: true,
455+
});
456+
457+
const state: HardwareWalletConnectionState = {
458+
status: ConnectionStatus.ErrorState,
459+
error: qrError,
460+
};
461+
462+
const result = getQrHardwareScanErrorAnalyticsProperties(state);
463+
expect(result).toEqual({
464+
error_category: 'ur_decode_error',
465+
is_ur_format: true,
466+
});
467+
});
468+
});
365469
});

app/core/HardwareWallet/analytics/helpers.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
HardwareWalletConnectionState,
55
ConnectionStatus,
66
} from '@metamask/hw-wallet-sdk';
7+
import { isQRHardwareScanError, QRHardwareScanErrorType } from '../errors';
78
import { ApprovalType } from '@metamask/controller-utils';
89
import { TransactionType } from '@metamask/transaction-controller';
910

@@ -186,3 +187,31 @@ export function getErrorDetails(
186187
}
187188
return { error_code: '', error_message: '' };
188189
}
190+
191+
/**
192+
* Segment/MetaMetrics properties for QR hardware camera scan failures
193+
* (`Hardware Wallet Connection Failed` / recovery UI), when the connection
194+
* {@link ConnectionStatus.ErrorState} error is a {@link isQRHardwareScanError}.
195+
*/
196+
export function getQrHardwareScanErrorAnalyticsProperties(
197+
connectionState: HardwareWalletConnectionState,
198+
): Record<string, string | boolean> {
199+
if (connectionState.status !== ConnectionStatus.ErrorState) {
200+
return {};
201+
}
202+
const { error } = connectionState;
203+
if (!isQRHardwareScanError(error)) {
204+
return {};
205+
}
206+
const metadata = error.metadata;
207+
const payload: Record<string, string | boolean> = {
208+
error_category: metadata.qrHardwareScanErrorType,
209+
is_ur_format: metadata.isUrFormat,
210+
};
211+
if (
212+
metadata.qrHardwareScanErrorType === QRHardwareScanErrorType.WrongURType
213+
) {
214+
payload.received_ur_type = metadata.receivedUrType ?? '';
215+
}
216+
return payload;
217+
}

app/core/HardwareWallet/analytics/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export {
66
getErrorTypeFromConnectionState,
77
getAnalyticsDeviceType,
88
getErrorDetails,
9+
getQrHardwareScanErrorAnalyticsProperties,
910
} from './helpers';
1011

1112
export { useHardwareWalletAnalytics } from './useHardwareWalletAnalytics';

app/core/HardwareWallet/analytics/useHardwareWalletAnalytics.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import {
1414
HardwareWalletAnalyticsFlow,
1515
} from './helpers';
1616
import { MetaMetricsEvents } from '../../Analytics';
17+
import { createQRHardwareScanError, QRHardwareScanErrorType } from '../errors';
18+
import { QrScanRequestType } from '@metamask/eth-qr-keyring';
1719

1820
const mockTrackEvent = jest.fn();
1921
const mockBuild = jest.fn().mockReturnValue({ name: 'built-event' });
@@ -86,6 +88,67 @@ describe('useHardwareWalletAnalytics', () => {
8688
expect(mockTrackEvent).toHaveBeenCalled();
8789
});
8890

91+
it('includes QR scan analytics when error is a QR hardware scan failure', () => {
92+
const { rerender } = renderHook(
93+
(props) => useHardwareWalletAnalytics(props),
94+
{ initialProps: defaultOptions },
95+
);
96+
97+
const qrError = createQRHardwareScanError({
98+
errorType: QRHardwareScanErrorType.NonURQrScanned,
99+
purpose: QrScanRequestType.PAIR,
100+
isUrFormat: false,
101+
});
102+
103+
rerender({
104+
...defaultOptions,
105+
walletType: HardwareWalletType.Qr,
106+
connectionState: {
107+
status: ConnectionStatus.ErrorState,
108+
error: qrError,
109+
},
110+
});
111+
112+
expect(mockAddProperties).toHaveBeenCalledWith(
113+
expect.objectContaining({
114+
error_type: HardwareWalletAnalyticsErrorType.GenericError,
115+
error_category: 'non_ur_qr_scanned',
116+
is_ur_format: false,
117+
}),
118+
);
119+
});
120+
121+
it('includes received_ur_type for wrong UR type QR scan errors', () => {
122+
const { rerender } = renderHook(
123+
(props) => useHardwareWalletAnalytics(props),
124+
{ initialProps: defaultOptions },
125+
);
126+
127+
const qrError = createQRHardwareScanError({
128+
errorType: QRHardwareScanErrorType.WrongURType,
129+
purpose: QrScanRequestType.SIGN,
130+
receivedUrType: 'eth-signature',
131+
isUrFormat: true,
132+
});
133+
134+
rerender({
135+
...defaultOptions,
136+
walletType: HardwareWalletType.Qr,
137+
connectionState: {
138+
status: ConnectionStatus.ErrorState,
139+
error: qrError,
140+
},
141+
});
142+
143+
expect(mockAddProperties).toHaveBeenCalledWith(
144+
expect.objectContaining({
145+
error_category: 'wrong_ur_type',
146+
is_ur_format: true,
147+
received_ur_type: 'eth-signature',
148+
}),
149+
);
150+
});
151+
89152
it('fires when transitioning to AwaitingApp', () => {
90153
const { rerender } = renderHook(
91154
(props) => useHardwareWalletAnalytics(props),

0 commit comments

Comments
 (0)