-
Notifications
You must be signed in to change notification settings - Fork 5.5k
feat: hardware wallet error provider #39391
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
montelaidev
wants to merge
58
commits into
main
Choose a base branch
from
feat/hardware-wallet-state-manager-6
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,447
−448
Open
Changes from all commits
Commits
Show all changes
58 commits
Select commit
Hold shift + click to select a range
53516ac
feat: add useHardwareWalletConnection Hook
montelaidev 7a27345
fix: test
montelaidev dc9d947
fix: device id reference
montelaidev 61b5f88
fix: remove test because of missing adapters
montelaidev bf59bef
fix: refactor use mock adapter
montelaidev e148ca9
fix: test
montelaidev 6c1ecf4
chore: fix message
montelaidev 46e78d2
fix: refactor remove reason from error state, and user hardware error
montelaidev 0df5923
fix: remove perido
montelaidev 39e1177
fix: use abort controller
montelaidev 1932d24
fix: update to use abort controller
montelaidev f37da82
fix: test
montelaidev ac0b07e
refactor: make mock simple
montelaidev 5404731
fix: lint
montelaidev ecce850
fix: race conditons
montelaidev 544cd8a
fix: use connectingPromiseRef for race-safe connection mutex
montelaidev 2226321
fix: remove error patterns
montelaidev 2794ea0
fix: remove incorrect typeguard
montelaidev 5d7a203
Merge remote-tracking branch 'origin/main' into feat/hardware-wallet-…
montelaidev ec14495
fix: lint
montelaidev c2cdaab
fix: update errors
montelaidev 4c0fb81
fix: tests
montelaidev bc42ebd
fix: remove try catch
montelaidev 05c1a14
fix: ref
montelaidev 6edb80b
fix: ref usage and test
montelaidev aa269d2
fix: lint
montelaidev b6721eb
fix: add additiaonl check
montelaidev cd4025d
Merge branch 'main' into feat/hardware-wallet-state-manager-5
montelaidev 8898972
Merge remote-tracking branch 'origin/main' into feat/hardware-wallet-…
montelaidev 9984c33
feat: hardware wallet state manager
montelaidev be7027b
fix: missing setters and duplicate previousWalletTypeRef
montelaidev 2a58b8b
fix: rebase on state context
montelaidev 6e029c8
fix: address comments
montelaidev b901b8a
fix: lint
montelaidev 1b6dca3
fix: update console baseline
montelaidev 9e83464
fix: lint
montelaidev dfbac92
fix: lint
montelaidev f8ca382
fix: add counter
montelaidev 4b18a97
fix: null reference if there is another connect while connecting
montelaidev 618553f
Merge remote-tracking branch 'origin/feat/hardware-wallet-state-manag…
montelaidev eab7d11
fix:race condition
montelaidev a0359bc
fix: update mock
montelaidev 2751f53
fix: remove ref from dep array
montelaidev 77fa31a
fix: add more guards, and reuse promise
montelaidev 86a97da
fix: lint
montelaidev fbe4615
Merge remote-tracking branch 'origin/feat/hardware-wallet-state-manag…
montelaidev 2a16d7a
Merge remote-tracking branch 'origin/main' into feat/hardware-wallet-…
montelaidev 1295fcd
Merge remote-tracking branch 'origin/feat/hardware-wallet-state-manag…
montelaidev 7acf3e0
fix: lint
montelaidev a8b09c8
fix: refs and add reset
montelaidev 6f1bc76
Merge remote-tracking branch 'origin/feat/hardware-wallet-state-manag…
montelaidev 653228a
fix: tests
montelaidev c7571ef
Merge remote-tracking branch 'origin/main' into feat/hardware-wallet-…
montelaidev f4bc32d
fix: refactor to have reset function and add rpc utils
montelaidev eb9889d
fix:add constant name
montelaidev 421c9d5
fix: lint
montelaidev c675486
fix: snapshot
montelaidev 4604558
fix: relax struct
montelaidev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
741 changes: 371 additions & 370 deletions
741
test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json
Large diffs are not rendered by default.
Oops, something went wrong.
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
267 changes: 267 additions & 0 deletions
267
ui/contexts/hardware-wallets/HardwareWalletErrorProvider.test.tsx
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,267 @@ | ||
| import React from 'react'; | ||
| import { render, act } from '@testing-library/react'; | ||
| import { renderHook } from '@testing-library/react-hooks'; | ||
| import { Provider } from 'react-redux'; | ||
| import configureStore from 'redux-mock-store'; | ||
| import { KeyringTypes } from '@metamask/keyring-controller'; | ||
| import { ErrorCode } from '@metamask/hw-wallet-sdk'; | ||
| import { | ||
| showModal, | ||
| hideModal, | ||
| setPendingHardwareWalletSigning, | ||
| closeCurrentNotificationWindow, | ||
| } from '../../store/actions'; | ||
| import { createHardwareWalletError } from './errors'; | ||
| import { | ||
| HardwareWalletErrorProvider, | ||
| useHardwareWalletError, | ||
| } from './HardwareWalletErrorProvider'; | ||
| import { HardwareWalletType } from './types'; | ||
| import { HARDWARE_WALLET_ERROR_MODAL_NAME } from './constants'; | ||
|
|
||
| const mockStore = configureStore([]); | ||
|
|
||
| jest.mock('../../store/actions'); | ||
| jest.mock('./HardwareWalletContext', () => ({ | ||
| HardwareWalletProvider: ({ children }: { children: React.ReactNode }) => | ||
| children, | ||
| useHardwareWalletConfig: () => ({ isHardwareWalletAccount: true }), | ||
| useHardwareWalletState: () => ({ | ||
| connectionState: { status: 'ready' }, | ||
| }), | ||
| useHardwareWalletActions: () => ({ | ||
| ensureDeviceReady: jest.fn().mockResolvedValue(true), | ||
| clearError: jest.fn(), | ||
| }), | ||
| })); | ||
|
|
||
| const mockShowModal = showModal as jest.Mock; | ||
| const mockHideModal = hideModal as jest.Mock; | ||
| const mocksetPendingHardwareWalletSigning = | ||
| setPendingHardwareWalletSigning as jest.Mock; | ||
| const mockCloseCurrentNotificationWindow = | ||
| closeCurrentNotificationWindow as jest.Mock; | ||
|
|
||
| // Mock showModal to return a proper action object | ||
| mockShowModal.mockImplementation((payload) => ({ | ||
| type: 'MODAL_OPEN', | ||
| payload, | ||
| })); | ||
|
|
||
| // Mock hideModal to return a proper action object | ||
| mockHideModal.mockImplementation(() => ({ | ||
| type: 'MODAL_CLOSE', | ||
| })); | ||
|
|
||
| mocksetPendingHardwareWalletSigning.mockImplementation((payload) => ({ | ||
| type: 'SET_PENDING_HARDWARE_SIGNING', | ||
| payload, | ||
| })); | ||
|
|
||
| mockCloseCurrentNotificationWindow.mockImplementation(() => ({ | ||
| type: 'CLOSE_NOTIFICATION_WINDOW', | ||
| })); | ||
|
|
||
| const createMockState = ( | ||
| keyringType: string | null = KeyringTypes.ledger, | ||
| address = '0x123', | ||
| ) => ({ | ||
| metamask: { | ||
| internalAccounts: { | ||
| accounts: { | ||
| 'account-1': { | ||
| id: 'account-1', | ||
| address, | ||
| metadata: { | ||
| keyring: { | ||
| type: keyringType, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| selectedAccount: 'account-1', | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| const createWrapper = | ||
| (store: ReturnType<typeof mockStore>) => | ||
| ({ children }: { children: React.ReactNode }) => ( | ||
| <Provider store={store}> | ||
| <HardwareWalletErrorProvider>{children}</HardwareWalletErrorProvider> | ||
| </Provider> | ||
| ); | ||
|
|
||
| const renderHardwareWalletErrorHook = (store: ReturnType<typeof mockStore>) => | ||
| renderHook(() => useHardwareWalletError(), { | ||
| wrapper: createWrapper(store), | ||
| }); | ||
|
|
||
| describe('HardwareWalletErrorProvider', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| describe('useHardwareWalletError', () => { | ||
| it('throws error when used outside provider', () => { | ||
| const HookConsumer = () => { | ||
| useHardwareWalletError(); | ||
| return null; | ||
| }; | ||
|
|
||
| const consoleError = jest | ||
| .spyOn(console, 'error') | ||
| .mockImplementation(() => undefined); | ||
|
|
||
| try { | ||
| expect(() => render(<HookConsumer />)).toThrow( | ||
| 'useHardwareWalletError must be used within HardwareWalletErrorProvider', | ||
| ); | ||
| } finally { | ||
| consoleError.mockRestore(); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe('error modal functionality', () => { | ||
| it('shows error modal when showErrorModal is called', () => { | ||
| const store = mockStore(createMockState()); | ||
| const { result } = renderHardwareWalletErrorHook(store); | ||
|
|
||
| const error = createHardwareWalletError( | ||
| ErrorCode.AuthenticationDeviceLocked, | ||
| HardwareWalletType.Ledger, | ||
| 'Device is locked', | ||
| ); | ||
|
|
||
| act(() => { | ||
| result.current.showErrorModal(error); | ||
| }); | ||
|
|
||
| expect(mockShowModal).toHaveBeenCalledWith({ | ||
| name: HARDWARE_WALLET_ERROR_MODAL_NAME, | ||
| error, | ||
| onRetry: expect.any(Function), | ||
| onCancel: expect.any(Function), | ||
| isOpen: true, | ||
| }); | ||
| }); | ||
|
|
||
| it('dismisses error modal when dismissErrorModal is called', () => { | ||
| const store = mockStore(createMockState()); | ||
| const { result } = renderHardwareWalletErrorHook(store); | ||
|
|
||
| const error = createHardwareWalletError( | ||
| ErrorCode.AuthenticationDeviceLocked, | ||
| HardwareWalletType.Ledger, | ||
| 'Device is locked', | ||
| ); | ||
|
|
||
| // First show a modal | ||
| act(() => { | ||
| result.current.showErrorModal(error); | ||
| }); | ||
|
|
||
| // Then dismiss it | ||
| act(() => { | ||
| result.current.dismissErrorModal(); | ||
| }); | ||
|
|
||
| expect(mockHideModal).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('reports modal visibility state', () => { | ||
| const store = mockStore(createMockState()); | ||
| const { result } = renderHardwareWalletErrorHook(store); | ||
|
|
||
| const error = createHardwareWalletError( | ||
| ErrorCode.AuthenticationDeviceLocked, | ||
| HardwareWalletType.Ledger, | ||
| 'Device is locked', | ||
| ); | ||
|
|
||
| expect(result.current.isErrorModalVisible).toBe(false); | ||
|
|
||
| act(() => { | ||
| result.current.showErrorModal(error); | ||
| }); | ||
|
|
||
| expect(result.current.isErrorModalVisible).toBe(true); | ||
|
|
||
| act(() => { | ||
| result.current.dismissErrorModal(); | ||
| }); | ||
|
|
||
| expect(result.current.isErrorModalVisible).toBe(false); | ||
| }); | ||
|
|
||
| it('calls onRetry callback when retry is triggered', async () => { | ||
| const store = mockStore(createMockState()); | ||
| const { result } = renderHardwareWalletErrorHook(store); | ||
|
|
||
| const error = createHardwareWalletError( | ||
| ErrorCode.AuthenticationDeviceLocked, | ||
| HardwareWalletType.Ledger, | ||
| 'Device is locked', | ||
| ); | ||
|
|
||
| act(() => { | ||
| result.current.showErrorModal(error); | ||
| }); | ||
|
|
||
| const { onRetry } = (showModal as jest.Mock).mock.calls[0][0]; | ||
|
|
||
| await act(async () => { | ||
| await onRetry(); | ||
| }); | ||
|
|
||
| expect(hideModal).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('calls onCancel callback when cancel is triggered', () => { | ||
| const store = mockStore(createMockState()); | ||
| const { result } = renderHardwareWalletErrorHook(store); | ||
|
|
||
| const error = createHardwareWalletError( | ||
| ErrorCode.AuthenticationDeviceLocked, | ||
| HardwareWalletType.Ledger, | ||
| 'Device is locked', | ||
| ); | ||
|
|
||
| act(() => { | ||
| result.current.showErrorModal(error); | ||
| }); | ||
|
|
||
| const { onCancel } = (showModal as jest.Mock).mock.calls[0][0]; | ||
|
|
||
| act(() => { | ||
| onCancel(); | ||
| }); | ||
|
|
||
| expect(hideModal).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('shows modal for user cancellation errors when called manually', () => { | ||
| const store = mockStore(createMockState()); | ||
| const { result } = renderHardwareWalletErrorHook(store); | ||
|
|
||
| const userCancelError = createHardwareWalletError( | ||
| ErrorCode.UserCancelled, | ||
| HardwareWalletType.Ledger, | ||
| 'User cancelled', | ||
| ); | ||
|
|
||
| act(() => { | ||
| result.current.showErrorModal(userCancelError); | ||
| }); | ||
|
|
||
| expect(mockShowModal).toHaveBeenCalledWith({ | ||
| name: HARDWARE_WALLET_ERROR_MODAL_NAME, | ||
| error: userCancelError, | ||
| onRetry: expect.any(Function), | ||
| onCancel: expect.any(Function), | ||
| isOpen: true, | ||
| }); | ||
| }); | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }); | ||
| }); | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I think
toBeDefinedwould sounds better giving the test description