Skip to content

feat: add new hardware wallet signing qr code#29737

Merged
montelaidev merged 9 commits into
mainfrom
feat/qr-siginig-1
May 6, 2026
Merged

feat: add new hardware wallet signing qr code#29737
montelaidev merged 9 commits into
mainfrom
feat/qr-siginig-1

Conversation

@montelaidev
Copy link
Copy Markdown
Contributor

@montelaidev montelaidev commented May 5, 2026

Description

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

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

Related issues

Refs: MUL-1665

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

N/A

After

N/A

Pre-merge author checklist

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

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
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).

Reviewed by Cursor Bugbot for commit 31443d4. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

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.

@github-actions 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 montelaidev self-assigned this May 5, 2026
@montelaidev montelaidev added the team-accounts-framework Accounts team label May 5, 2026
@montelaidev montelaidev marked this pull request as ready for review May 5, 2026 13:58
@github-actions github-actions Bot added the size-L label May 5, 2026
Comment thread app/core/HardwareWallet/HardwareWalletProvider.tsx
@montelaidev montelaidev enabled auto-merge May 5, 2026 14:26
@github-actions github-actions Bot added size-XL and removed size-L labels May 6, 2026
Comment thread app/core/HardwareWallet/hooks/useQrConfirm.ts Outdated
Comment thread app/core/HardwareWallet/hooks/useQrConfirm.ts
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.12903% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.98%. Comparing base (8437791) to head (eb05c39).
⚠️ Report is 103 commits behind head on main.

Files with missing lines Patch % Lines
app/core/HardwareWallet/HardwareWalletProvider.tsx 23.07% 10 Missing ⚠️
app/core/HardwareWallet/hooks/useQrConfirm.ts 96.77% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #29737      +/-   ##
==========================================
+ Coverage   81.86%   81.98%   +0.11%     
==========================================
  Files        5255     5296      +41     
  Lines      138980   140318    +1338     
  Branches    31518    31901     +383     
==========================================
+ Hits       113774   115033    +1259     
- Misses      17465    17479      +14     
- Partials     7741     7806      +65     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread app/core/HardwareWallet/adapters/QRWalletAdapter.ts
Copy link
Copy Markdown
Contributor

@david0xd david0xd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good! I haven't tested due to limitations of the iOS simulator and incapabilities (slowness) of my Android device. Please find someone else to test it manually 🙏

@montelaidev
Copy link
Copy Markdown
Contributor Author

Code looks good! I haven't tested due to limitations of the iOS simulator and incapabilities (slowness) of my Android device. Please find someone else to test it manually 🙏

The code is mainly dead code and will only be used in the subsequent pr

Attached is a video of the signing

Screen_Recording_20260506_195134_MetaMask.mp4

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Fix All in Cursor

❌ 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 c210658. Configure here.

import {
createQRHardwareScanError,
QRHardwareScanErrorType,
isQRHardwareScanError as actualIsQRHardwareScanError,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import actualIsQRHardwareScanError in test file

Low Severity

isQRHardwareScanError as actualIsQRHardwareScanError is imported but never used anywhere in the test file. This is dead code that adds confusion — it may suggest a missing assertion or test that was intended but forgotten.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c210658. Configure here.

@montelaidev 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 6, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeAccounts, SmokeConfirmations
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 82%
click to see 🤖 AI reasoning details

E2E Test Selection:
All 25 changed files are within app/core/HardwareWallet/ and relate to QR-based hardware wallet functionality improvements:

  1. SmokeAccounts: The tag description explicitly covers "adding QR-based hardware wallet accounts." The changes to QRWalletAdapter (camera permission logic, removed spurious AppOpened event), HardwareWalletProvider (new retry handler), and the new useIsConfirmationFromQrAccount hook all affect the QR hardware wallet account connection and management flow. These changes could affect how QR hardware wallet accounts are added and managed.

  2. SmokeConfirmations: The new useQrConfirm hook directly integrates with the confirmations system (useApprovalRequest, useTransactionMetadataRequest) to coordinate QR hardware wallet confirmation for both transactions and message approvals. Changes to AwaitingConfirmationContent (auto-open scanner on retry), ErrorContent (redesigned error UI with Try Again/Learn More), and the overall retry flow all affect the confirmation experience for QR hardware wallet accounts. The SmokeConfirmations tag covers signature requests, transaction sending, and smart contract interactions - all of which can be initiated from QR hardware wallet accounts.

No performance-sensitive paths are affected - these are UI/UX and error handling improvements to the hardware wallet flow. No performance tests are warranted.

The changes are medium risk: they modify existing behavior (QRWalletAdapter camera permission logic, error handling flow) and add new retry coordination logic that could affect the QR hardware wallet confirmation flow. However, the scope is limited to the HardwareWallet module with no impact on core wallet infrastructure.

Performance Test Selection:
All changes are within the HardwareWallet module and focus on UI/UX improvements, error handling, and retry flow coordination for QR hardware wallets. There are no changes to performance-sensitive paths like rendering lists, data loading, state management at scale, or app initialization. No performance tests are warranted.

View GitHub Actions results

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 6, 2026

@montelaidev montelaidev added this pull request to the merge queue May 6, 2026
Merged via the queue into main with commit 5dd99a1 May 6, 2026
93 of 96 checks passed
@montelaidev montelaidev deleted the feat/qr-siginig-1 branch May 6, 2026 13:57
@github-actions github-actions Bot locked and limited conversation to collaborators May 6, 2026
@metamaskbotv2 metamaskbotv2 Bot added the release-7.77.0 Issue or pull request that will be included in release 7.77.0 label May 6, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.77.0 Issue or pull request that will be included in release 7.77.0 size-XL team-accounts-framework Accounts team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants