feat: wire qr code errors by montelaidev · Pull Request #29741 · MetaMask/metamask-mobile

montelaidev · 2026-05-05T14:25:22Z

Description

Wires the QR scan errors from PR 2 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 3 of 3 and is stacked on #29388.

Changelog

CHANGELOG entry: Improved retry behavior when QR hardware wallet signing scans fail

Related issues

Refs: MUL-1665
Resolves: https://consensyssoftware.atlassian.net/browse/MUL-1513?atlOrigin=eyJpIjoiZThlODA2MTIwODU4NDA5ZmI1YjdlMjJkM2Q2ODdlOWQiLCJwIjoiaiJ9

Manual testing steps

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

Before

After

Screen_Recording_20260512_222018_MetaMask.mp4

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
I've completed the PR template to the best of my ability
I've included tests if applicable
I've documented my code using JSDoc format if applicable
I've applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Performance checks (if applicable)

I've tested on Android
- Ideally on a mid-range device; emulator is acceptable
I've tested with a power user scenario
- Use these power-user SRPs to import wallets with many accounts and tokens
I've instrumented key operations with Sentry traces for production performance metrics
- See trace() for usage and addToken for an example

For performance guidelines and tooling, see the Performance Guide.

Pre-merge reviewer checklist

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

Note

Medium Risk
Touches QR hardware-wallet signing/pairing flows and error-handling/retry behavior, which can impact user ability to connect/sign if regressions occur. Changes are mostly additive/guarded and covered by expanded unit tests, but they alter control flow around modal visibility and error deduping.

Overview
Routes QR-hardware scan failures (non-UR, decode errors, wrong UR type, scan exceptions) out of AnimatedQRScannerModal into the Hardware Wallet bottom sheet via a new useQrScanErrorForwarding hook, enabling Try again retry to reopen scanners in pairing (ConnectQRHardware), signing (QRSigningDetails), and confirmation QR signing (QRInfo).

Hardens scanner behavior by deduplicating repeated error frames (isSameScanError), avoiding repeated decoder resets/analytics, and resetting decoder/dedup state on visible transitions; also makes analytics sending resilient to failures.

Improves QR transport handling by centralizing camera permission status constants (CAMERA_PERMISSION_STATUS) and updating QRWalletAdapter to request permission for any non-granted status; adds adapter factory support for HardwareWalletType.Qr and normalizes replacement-tx gas param typing via shared ReplacementTxParams.

^{Reviewed by Cursor Bugbot for commit 49f5ad6. Bugbot is set up for automated code reviews on this repo. Configure here.}

github-actions · 2026-05-05T14:25:45Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

david0xd · 2026-05-12T14:07:30Z

I've reviewed the code. Changes look decent and reasonable. I haven't tested, so just please find someone to do some additional manual testing if needed.

owencraston · 2026-05-12T23:22:02Z

@montelaidev this change seems to be working during onboarding and with dapp signing but for sending and swapping its really messed up. I think these issues might stem for this bug but either way I am seeing really weird results...

first it shows the qr code in full screen mode
then it shows the qr code in popup mode
the transaction has to be signed multiple times.

Here is a video:

screen-20260512-190410-1778626845710.2.mp4

github-actions · 2026-05-13T09:25:14Z

🔍 Smart E2E Test Selection

Selected E2E tags: SmokeAccounts, SmokeConfirmations, SmokeIdentity, SmokeNetworkAbstractions, SmokeNetworkExpansion, SmokeSwap, SmokeStake, SmokeWalletPlatform, SmokeMoney, SmokePerps, SmokeMultiChainAPI, SmokePredictions, SmokeSeedlessOnboarding, SmokeBrowser, SmokeSnaps
Selected Performance tags: @PerformanceAccountList, @PerformanceOnboarding, @PerformanceLogin, @PerformanceSwaps, @PerformanceLaunch, @PerformanceAssetLoading, @PerformancePredict, @PerformancePreps
Risk Level: high
AI Confidence: %

click to see 🤖 AI reasoning details

E2E Test Selection:
Fallback: AI analysis did not complete successfully. Running all tests.

Performance Test Selection:
Fallback: AI analysis did not complete successfully. Running all performance tests.

View GitHub Actions results

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 49f5ad6. Configure here.}

cursor · 2026-05-13T09:34:56Z

+    return () => {
+      setQrScanRetryHandler?.(null);
+    };
+  }, [setQrScanRetryHandler, showScanner]);


Signing retry doesn't reopen QR scanner

High Severity

The QR scan retry handlers registered in QRSigningDetails and QRInfo (which call showScanner()/setScannerVisible(true)) are never invoked during signing flows. When handleRetryQrScan in the provider runs and operationTypeRef.current is not null (signing is in progress), it transitions to AwaitingConfirmation and returns early without calling qrScanRetryHandlerRef.current(). The scanner stays closed, leaving the user stuck on the awaiting-confirmation bottom sheet with no way to re-scan. This contradicts the PR's stated goal that "signing confirmations can reopen the scanner from the error CTA."

Additional Locations (1)

app/components/Views/confirmations/components/qr-info/qr-info.tsx#L42-L51

^{Reviewed by Cursor Bugbot for commit 49f5ad6. Configure here.}

sonarqubecloud · 2026-05-13T09:40:44Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
98.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

montelaidev · 2026-05-13T10:08:30Z

@montelaidev this change seems to be working during onboarding and with dapp signing but for sending and swapping its really messed up. I think these issues might stem for this bug but either way I am seeing really weird results...

first it shows the qr code in full screen mode

then it shows the qr code in popup mode

the transaction has to be signed multiple times.

Here is a video:

screen-20260512-190410-1778626845710.2.mp4

Will be fixed in this pr

montelaidev added 4 commits May 5, 2026 21:54

feat: add new hardware wallet signing qr code

c4f2c1c

fix: remove unused code

30e4dc5

fix: show qr signing error

90c5191

fix: add retry to bottom sheet

b7b6940

montelaidev requested review from a team as code owners May 5, 2026 14:25

github-actions Bot added the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label May 5, 2026

montelaidev changed the base branch from main to feat/qr-siginig-1 May 5, 2026 14:25

montelaidev self-assigned this May 5, 2026

montelaidev added the team-accounts-framework Accounts team label May 5, 2026

github-actions Bot added the size-XL label May 5, 2026

montelaidev changed the title ~~Feat/qr siginig 2~~ feat: wire qr code errors May 5, 2026

fix: lint

f8dbaf8

cursor Bot reviewed May 5, 2026

View reviewed changes

Comment thread app/components/Views/ConnectQRHardware/index.tsx

montelaidev mentioned this pull request May 6, 2026

feat: add new hardware wallet signing qr code #29737

Merged

10 tasks

Base automatically changed from feat/qr-siginig-1 to main May 6, 2026 13:57

Merge remote-tracking branch 'origin/main' into feat/qr-siginig-2

d408d6c

cursor Bot reviewed May 6, 2026

View reviewed changes

Comment thread app/core/HardwareWallet/adapters/QRWalletAdapter.test.ts Outdated

Comment thread app/core/HardwareWallet/hooks/useQrConfirm.ts Outdated

montelaidev added 4 commits May 6, 2026 23:21

fix: coverage

0b40307

fix: tests and lint

5f63db3

fix: add coverage

445baa7

Merge branch 'main' into feat/qr-siginig-2

2fffdff

cursor Bot reviewed May 7, 2026

View reviewed changes

Comment thread app/components/UI/QRHardware/QRSigningDetails.tsx

montelaidev added 2 commits May 7, 2026 23:44

fix: test

b934a75

fix: callback

907b880

montelaidev removed the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label May 11, 2026

Merge branch 'main' into feat/qr-siginig-2

4144c85

ccharly reviewed May 11, 2026

View reviewed changes

Comment thread app/components/UI/QRHardware/AnimatedQRScanner.tsx

david0xd reviewed May 11, 2026

View reviewed changes

Comment thread app/components/UI/QRHardware/AnimatedQRScanner.utils.ts

david0xd reviewed May 11, 2026

View reviewed changes

Comment thread app/core/HardwareWallet/adapters/QRWalletAdapter.ts Outdated

montelaidev added 2 commits May 12, 2026 14:49

fix: update jsdoc

a5fccca

fix: make reset explicit

5f9c7b2

cursor Bot reviewed May 12, 2026

View reviewed changes

Comment thread app/components/UI/QRHardware/AnimatedQRScanner.tsx

montelaidev added 2 commits May 12, 2026 17:48

fix: lint and duplicate events

83f4042

fix: test

47feef5

cursor Bot reviewed May 12, 2026

View reviewed changes

Comment thread app/components/Views/confirmations/components/qr-info/qr-info.tsx

fix: remove inline function

ed1d75d

david0xd approved these changes May 12, 2026

View reviewed changes

OGPoyraz approved these changes May 13, 2026

View reviewed changes

Merge branch 'main' into feat/qr-siginig-2

49f5ad6

cursor Bot reviewed May 13, 2026

View reviewed changes

PatrykLucka approved these changes May 13, 2026

View reviewed changes

montelaidev added this pull request to the merge queue May 13, 2026

Merged via the queue into main with commit 34ffb5c May 13, 2026
115 of 116 checks passed

montelaidev deleted the feat/qr-siginig-2 branch May 13, 2026 11:06

github-actions Bot locked and limited conversation to collaborators May 13, 2026

metamaskbotv2 Bot added the release-7.78.0 Issue or pull request that will be included in release 7.78.0 label May 13, 2026

Uh oh!

Conversation

montelaidev commented May 5, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Performance checks (if applicable)

Pre-merge reviewer checklist

Uh oh!

github-actions Bot commented May 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

david0xd commented May 12, 2026

Uh oh!

owencraston commented May 12, 2026

Uh oh!

github-actions Bot commented May 13, 2026

🔍 Smart E2E Test Selection

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot May 13, 2026

Choose a reason for hiding this comment

Signing retry doesn't reopen QR scanner

Uh oh!

sonarqubecloud Bot commented May 13, 2026

Quality Gate passed

Uh oh!

montelaidev commented May 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

montelaidev commented May 5, 2026 •

edited by cursor Bot

Loading