Skip to content

Commit cc1992b

Browse files
authored
feat(hw): add executeHardwareWalletOperation utility (#29085)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Add shared utility that runs hardware wallet operations behind the standard readiness + awaiting-confirmation flow. Includes foundation helpers (getDeviceIdForAddress, normalizeReplacementGasFeeParams) as prerequisites. <!-- 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? --> ## **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: null ## **Related issues** Related to: https://consensyssoftware.atlassian.net/browse/MUL-1741 ## **Manual testing steps** This can only be tested via #29087 ## **Screenshots/Recordings** Not applicable ## **Pre-merge author checklist** - [ ] 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). - [ ] 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](https://jsdoc.app/) format if applicable - [ ] 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) - [ ] 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](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 - [ ] 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** - [ ] 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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches the hardware wallet provider’s wallet-type resolution and introduces a new shared signing wrapper; mistakes could lead to incorrect adapter selection or confirmation/error UI behavior during signing. > > **Overview** > Adds a shared `executeHardwareWalletOperation` helper to standardize hardware-wallet operations: it resolves device id from address, gates on `ensureDeviceReady`, shows/hides the awaiting-confirmation UI, and centralizes rejection + error handling (including user-cancel suppression), with unit coverage. > > Updates `HardwareWalletProvider`/context to support a new `setPendingOperationAddress` hook and to *auto-derive* the effective wallet type from the in-flight operation address (via `getHardwareWalletTypeForAddress`) when no explicit `targetWalletType` is set. Also tightens the `setTargetWalletType` context type and updates affected tests/mocks. > > Separately introduces a `ReplacementGasFeeValues` type alias and reuses it in replacement gas normalization/repricing helpers (no functional behavior change). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3db6970. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 2e461bc commit cc1992b

8 files changed

Lines changed: 379 additions & 5 deletions

File tree

app/components/Views/LedgerSelectAccount/index.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ const defaultHardwareWalletValues = {
163163
},
164164
ensureDeviceReady: mockEnsureDeviceReady,
165165
setTargetWalletType: mockSetTargetWalletType,
166+
setPendingOperationAddress: jest.fn(),
166167
showHardwareWalletError: mockShowHardwareWalletError,
167168
showAwaitingConfirmation: mockShowAwaitingConfirmation,
168169
hideAwaitingConfirmation: mockHideAwaitingConfirmation,

app/core/HardwareWallet/HardwareWalletProvider.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import React, {
88

99
import HardwareWalletContext from './contexts/HardwareWalletContext';
1010
import { HardwareWalletBottomSheet } from './components';
11+
import { getHardwareWalletTypeForAddress } from './helpers';
1112
import {
1213
useHardwareWalletStateManager,
1314
useDeviceEventHandlers,
@@ -42,7 +43,16 @@ export const HardwareWalletProvider: React.FC<HardwareWalletProviderProps> = ({
4243
const { state, refs, setters } = useHardwareWalletStateManager();
4344
const { connectionState, deviceId, walletType, targetWalletType } = state;
4445

45-
const effectiveWalletType = targetWalletType ?? walletType;
46+
const [pendingOperationAddress, setPendingOperationAddress] = useState<
47+
string | null
48+
>(null);
49+
50+
const walletTypeFromPendingAddress = pendingOperationAddress
51+
? (getHardwareWalletTypeForAddress(pendingOperationAddress) ?? null)
52+
: null;
53+
54+
const effectiveWalletType =
55+
targetWalletType ?? walletTypeFromPendingAddress ?? walletType;
4656

4757
const { handleDeviceEvent, handleError, updateConnectionState } =
4858
useDeviceEventHandlers({
@@ -189,6 +199,7 @@ export const HardwareWalletProvider: React.FC<HardwareWalletProviderProps> = ({
189199
deviceSelection,
190200
ensureDeviceReady,
191201
setTargetWalletType: setters.setTargetWalletType,
202+
setPendingOperationAddress,
192203
showHardwareWalletError,
193204
showAwaitingConfirmation,
194205
hideAwaitingConfirmation,
@@ -200,6 +211,7 @@ export const HardwareWalletProvider: React.FC<HardwareWalletProviderProps> = ({
200211
deviceSelection,
201212
ensureDeviceReady,
202213
setters.setTargetWalletType,
214+
setPendingOperationAddress,
203215
showHardwareWalletError,
204216
showAwaitingConfirmation,
205217
hideAwaitingConfirmation,

app/core/HardwareWallet/contexts/HardwareWalletContext.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const createMockContextValue = (
2727
deviceSelection: defaultDeviceSelection,
2828
ensureDeviceReady: asyncFalse,
2929
setTargetWalletType: noop,
30+
setPendingOperationAddress: noop,
3031
showHardwareWalletError: noop,
3132
showAwaitingConfirmation: noop,
3233
hideAwaitingConfirmation: noop,

app/core/HardwareWallet/contexts/HardwareWalletContext.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ export interface HardwareWalletContextValue {
2222
*/
2323
ensureDeviceReady: (deviceId?: string | null) => Promise<boolean>;
2424
/** Set the target wallet type for "Add Hardware Wallet" flows (no account yet). */
25-
setTargetWalletType: (walletType: HardwareWalletType | null) => void;
25+
setTargetWalletType: (walletType: HardwareWalletType) => void;
26+
/** Set the pending operation address so the provider can auto-derive the wallet type during signing. */
27+
setPendingOperationAddress: (address: string | null) => void;
2628
/** Show a hardware wallet error in the bottom sheet. Use after ensureDeviceReady succeeds. */
2729
showHardwareWalletError: (error: unknown) => void;
2830
/** Show "awaiting confirmation" bottom sheet. */
Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
import { executeHardwareWalletOperation } from './executeHardwareWalletOperation';
2+
3+
const mockGetDeviceIdForAddress = jest.fn();
4+
const mockIsUserCancellation = jest.fn().mockReturnValue(false);
5+
6+
jest.mock('./helpers', () => ({
7+
getDeviceIdForAddress: (...args: unknown[]) =>
8+
mockGetDeviceIdForAddress(...args),
9+
}));
10+
11+
jest.mock('./errors', () => ({
12+
isUserCancellation: (...args: unknown[]) => mockIsUserCancellation(...args),
13+
}));
14+
15+
describe('executeHardwareWalletOperation', () => {
16+
const ensureDeviceReady = jest.fn();
17+
const showAwaitingConfirmation = jest.fn();
18+
const hideAwaitingConfirmation = jest.fn();
19+
const showHardwareWalletError = jest.fn();
20+
const setPendingOperationAddress = jest.fn();
21+
const execute = jest.fn();
22+
const onError = jest.fn();
23+
const onRejected = jest.fn();
24+
25+
const baseOptions = {
26+
address: '0x123',
27+
operationType: 'transaction' as const,
28+
ensureDeviceReady,
29+
setPendingOperationAddress,
30+
showAwaitingConfirmation,
31+
hideAwaitingConfirmation,
32+
showHardwareWalletError,
33+
onError,
34+
execute,
35+
onRejected,
36+
};
37+
38+
beforeEach(() => {
39+
jest.clearAllMocks();
40+
mockGetDeviceIdForAddress.mockResolvedValue('device-123');
41+
ensureDeviceReady.mockResolvedValue(true);
42+
execute.mockResolvedValue(undefined);
43+
onError.mockResolvedValue(false);
44+
});
45+
46+
it('resolves the device id before checking readiness', async () => {
47+
await expect(executeHardwareWalletOperation(baseOptions)).resolves.toBe(
48+
true,
49+
);
50+
51+
expect(mockGetDeviceIdForAddress).toHaveBeenCalledWith('0x123');
52+
expect(ensureDeviceReady).toHaveBeenCalledWith('device-123');
53+
});
54+
55+
it('sets the pending operation address at the start and clears it in the finally block', async () => {
56+
await executeHardwareWalletOperation(baseOptions);
57+
58+
expect(setPendingOperationAddress).toHaveBeenNthCalledWith(1, '0x123');
59+
expect(setPendingOperationAddress).toHaveBeenLastCalledWith(null);
60+
});
61+
62+
it('shows awaiting confirmation while executing the operation', async () => {
63+
await executeHardwareWalletOperation(baseOptions);
64+
65+
expect(showAwaitingConfirmation).toHaveBeenCalledWith(
66+
'transaction',
67+
expect.any(Function),
68+
);
69+
expect(execute).toHaveBeenCalledTimes(1);
70+
expect(hideAwaitingConfirmation).toHaveBeenCalledTimes(1);
71+
expect(onRejected).not.toHaveBeenCalled();
72+
});
73+
74+
it('rejects when the device is not ready', async () => {
75+
ensureDeviceReady.mockResolvedValue(false);
76+
77+
await expect(executeHardwareWalletOperation(baseOptions)).resolves.toBe(
78+
false,
79+
);
80+
81+
expect(showAwaitingConfirmation).not.toHaveBeenCalled();
82+
expect(execute).not.toHaveBeenCalled();
83+
expect(onRejected).toHaveBeenCalledTimes(1);
84+
});
85+
86+
it('shows the hardware wallet error for unexpected operation errors', async () => {
87+
const error = new Error('signing failed');
88+
execute.mockRejectedValueOnce(error);
89+
90+
await expect(executeHardwareWalletOperation(baseOptions)).resolves.toBe(
91+
false,
92+
);
93+
94+
expect(hideAwaitingConfirmation).toHaveBeenCalledTimes(1);
95+
expect(showHardwareWalletError).toHaveBeenCalledWith(error);
96+
expect(onRejected).toHaveBeenCalledTimes(1);
97+
});
98+
99+
it('lets callers handle non-user cancellations without showing the shared hardware wallet error', async () => {
100+
const error = new Error('keystone cancelled');
101+
execute.mockRejectedValueOnce(error);
102+
onError.mockResolvedValueOnce(true);
103+
104+
await expect(executeHardwareWalletOperation(baseOptions)).resolves.toBe(
105+
false,
106+
);
107+
108+
expect(onError).toHaveBeenCalledWith(error);
109+
expect(showHardwareWalletError).not.toHaveBeenCalled();
110+
expect(onRejected).toHaveBeenCalledTimes(1);
111+
});
112+
113+
it('does not show hardware wallet errors for user cancellations', async () => {
114+
const error = new Error('User rejected');
115+
execute.mockRejectedValueOnce(error);
116+
mockIsUserCancellation.mockReturnValueOnce(true);
117+
118+
await expect(executeHardwareWalletOperation(baseOptions)).resolves.toBe(
119+
false,
120+
);
121+
122+
expect(showHardwareWalletError).not.toHaveBeenCalled();
123+
expect(onRejected).toHaveBeenCalledTimes(1);
124+
});
125+
126+
it('rejects only once when the awaiting confirmation cancel callback fires after an error', async () => {
127+
let cancelCallback: (() => void) | undefined;
128+
showAwaitingConfirmation.mockImplementation(
129+
(_operationType: string, onReject?: () => void) => {
130+
cancelCallback = onReject;
131+
},
132+
);
133+
execute.mockRejectedValueOnce(new Error('fail'));
134+
135+
await expect(executeHardwareWalletOperation(baseOptions)).resolves.toBe(
136+
false,
137+
);
138+
cancelCallback?.();
139+
140+
expect(onRejected).toHaveBeenCalledTimes(1);
141+
});
142+
143+
it('works without setPendingOperationAddress provided', async () => {
144+
const { setPendingOperationAddress: _, ...optionsWithoutSetter } =
145+
baseOptions;
146+
147+
await expect(
148+
executeHardwareWalletOperation(optionsWithoutSetter),
149+
).resolves.toBe(true);
150+
151+
expect(execute).toHaveBeenCalledTimes(1);
152+
});
153+
154+
it('works without onRejected provided', async () => {
155+
const { onRejected: _, ...optionsWithoutReject } = baseOptions;
156+
157+
await expect(
158+
executeHardwareWalletOperation(optionsWithoutReject),
159+
).resolves.toBe(true);
160+
});
161+
162+
it('passes message operationType to showAwaitingConfirmation', async () => {
163+
await expect(
164+
executeHardwareWalletOperation({
165+
...baseOptions,
166+
operationType: 'message',
167+
}),
168+
).resolves.toBe(true);
169+
170+
expect(showAwaitingConfirmation).toHaveBeenCalledWith(
171+
'message',
172+
expect.any(Function),
173+
);
174+
});
175+
176+
it('skips hardware wallet error when cancel callback fires before execute throws', async () => {
177+
let cancelCallback: (() => void) | undefined;
178+
showAwaitingConfirmation.mockImplementation(
179+
(_operationType: string, onReject?: () => void) => {
180+
cancelCallback = onReject;
181+
},
182+
);
183+
execute.mockImplementation(async () => {
184+
cancelCallback?.();
185+
throw new Error('fail');
186+
});
187+
188+
await expect(executeHardwareWalletOperation(baseOptions)).resolves.toBe(
189+
false,
190+
);
191+
192+
expect(showHardwareWalletError).not.toHaveBeenCalled();
193+
expect(onRejected).toHaveBeenCalledTimes(1);
194+
});
195+
196+
it('shows the hardware wallet error when device id lookup fails', async () => {
197+
const error = new Error('device lookup failed');
198+
mockGetDeviceIdForAddress.mockRejectedValueOnce(error);
199+
200+
await expect(executeHardwareWalletOperation(baseOptions)).resolves.toBe(
201+
false,
202+
);
203+
204+
expect(showHardwareWalletError).toHaveBeenCalledWith(error);
205+
expect(execute).not.toHaveBeenCalled();
206+
});
207+
208+
it('does not call hideAwaitingConfirmation when the error occurs before showAwaitingConfirmation', async () => {
209+
const error = new Error('device lookup failed');
210+
mockGetDeviceIdForAddress.mockRejectedValueOnce(error);
211+
212+
await executeHardwareWalletOperation(baseOptions);
213+
214+
expect(showAwaitingConfirmation).not.toHaveBeenCalled();
215+
expect(hideAwaitingConfirmation).not.toHaveBeenCalled();
216+
});
217+
218+
it('does not call hideAwaitingConfirmation when ensureDeviceReady throws before showAwaitingConfirmation', async () => {
219+
const error = new Error('BLE connection failed');
220+
ensureDeviceReady.mockRejectedValueOnce(error);
221+
222+
await executeHardwareWalletOperation(baseOptions);
223+
224+
expect(showAwaitingConfirmation).not.toHaveBeenCalled();
225+
expect(hideAwaitingConfirmation).not.toHaveBeenCalled();
226+
});
227+
228+
it('clears pending operation address in finally block even on error', async () => {
229+
execute.mockRejectedValueOnce(new Error('fail'));
230+
231+
await expect(executeHardwareWalletOperation(baseOptions)).resolves.toBe(
232+
false,
233+
);
234+
235+
expect(setPendingOperationAddress).toHaveBeenNthCalledWith(1, '0x123');
236+
expect(setPendingOperationAddress).toHaveBeenLastCalledWith(null);
237+
});
238+
});

0 commit comments

Comments
 (0)