feat: hardware wallet error provider #39391
Open
+1,447
−448
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
Contributor
|
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. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
Contributor
Builds ready [b19aab3]
UI Startup Metrics (1308 ± 105 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
|
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
montelaidev
had a problem deploying
to
pr-comment
GitHub Actions
Error
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
Contributor
Builds ready [085dc27]
UI Startup Metrics (1293 ± 100 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
|
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
Contributor
Builds ready [4548db5]
UI Startup Metrics (1298 ± 113 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
|
…er-8' into feat/hardware-wallet-state-manager-6
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
Contributor
Builds ready [1295fcd]
UI Startup Metrics (1339 ± 109 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
|
…er-8' into feat/hardware-wallet-state-manager-6
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
Contributor
Builds ready [c7571ef]
UI Startup Metrics (1264 ± 132 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs
|
ccharly
reviewed
Jan 30, 2026
ui/contexts/hardware-wallets/HardwareWalletErrorProvider.test.tsx
Outdated
Show resolved
Hide resolved
ui/contexts/hardware-wallets/HardwareWalletErrorProvider.test.tsx
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,231 @@ | |||
| import React from 'react'; | |||
| import { renderHook, act } from '@testing-library/react-hooks'; | |||
Contributor
There was a problem hiding this comment.
Looks like that's what we use for other renderHook imports in the codebase (but maybe that's deprecated yes..)
IMO, let's keep react-hooks here, and we can unify all that later all-at-once
Comment on lines
43
to
60
| mockShowModal.mockImplementation((payload) => ({ | ||
| type: 'MODAL_OPEN', | ||
| payload, | ||
| })); | ||
|
|
||
| // Mock hideModal to return a proper action object | ||
| mockHideModal.mockImplementation(() => ({ | ||
| type: 'MODAL_CLOSE', | ||
| })); | ||
|
|
||
| mockSetPendingHardwareSigning.mockImplementation((payload) => ({ | ||
| type: 'SET_PENDING_HARDWARE_SIGNING', | ||
| payload, | ||
| })); | ||
|
|
||
| mockCloseCurrentNotificationWindow.mockImplementation(() => ({ | ||
| type: 'CLOSE_NOTIFICATION_WINDOW', | ||
| })); |
Contributor
There was a problem hiding this comment.
Nit: But we could re-use constant here too (if we have any)
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
montelaidev
commented
Jan 30, 2026
| @@ -1,123 +1,44 @@ | |||
| { | |||
| "DNS": "object", | |||
Contributor
Author
There was a problem hiding this comment.
Update snapshot changed this.
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
| setDisplayedError(null); | ||
| } | ||
| }; | ||
| }, [dispatch]); |
There was a problem hiding this comment.
Cleanup effect missing pendingHardwareSigning state reset
Medium Severity
The cleanup effect on unmount doesn't call dispatch(setPendingHardwareWalletSigning(false)), unlike resetModalState which is used by handleCancel, handleRetry, and dismissErrorModal. If the component unmounts while the modal is open (e.g., during navigation), the Redux state pendingHardwareSigning remains true, potentially preventing notification popups from auto-closing as intended by the selector getIsHardwareWalletErrorModalVisible.
montelaidev
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
Contributor
Builds ready [4604558]
UI Startup Metrics (1300 ± 119 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is the 6th of 11 prs to introduce system for handling hardware wallet connections in MetaMask. It manages the connection lifecycle, permission handling, error states, and provides a clean interface for UI components to interact with hardware wallets.
This PR adds the hardware error provider that will be responsible for displaying the preflight errors.
Depends on:
Changelog
CHANGELOG entry: null
Related issues
Related to: https://consensyssoftware.atlassian.net/browse/MUL-1299?atlOrigin=eyJpIjoiZWZlYjE4M2NiOWVmNDk0N2I3MzA4MzMzZTg2M2U1YzYiLCJwIjoiaiJ9
Manual testing steps
Not testable, this pr only introduces
Screenshots/Recordings
N/a. Not testable because it is not wired up.
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Adds new hardware-wallet error modal orchestration and new Redux state/actions that can affect approval/popup lifecycle. Also broadens RPC error deserialization/mapping logic, which could change how hardware-wallet failures are surfaced and retried.
Overview
Adds a new
HardwareWalletErrorProvider/useHardwareWalletErrorthat watchesHardwareWalletContextfor error states and dispatches aHARDWARE_WALLET_ERRORmodal with retry/cancel behavior, including special handling to suppress auto-modals for user-cancel/user-reject errors unless explicitly shown.Introduces
appState.pendingHardwareSigningwithSET_PENDING_HARDWARE_SIGNING+setPendingHardwareWalletSigning, plus selectors (getPendingHardwareSigning,getIsHardwareWalletErrorModalVisible) to help coordinate popup auto-close behavior while an error modal is open.Refactors
rpcErrorUtilsto robustly detect and reconstruct hardware-wallet errors across the RPC boundary (including serializeddata.causeshapes, string/numericErrorCodes,KeyringControllerError, and Ledger status-code mapping), and tightens connection cleanup inuseHardwareWalletConnectionto avoid nulling a newly replaced adapter. Updates/extends unit tests and the metrics e2e state snapshot accordingly.Written by Cursor Bugbot for commit 4604558. This will update automatically on new commits. Configure here.