Skip to content

Commit 040087d

Browse files
committed
[DDW-722] Review changes
1 parent da1c6bb commit 040087d

File tree

14 files changed

+78
-77
lines changed

14 files changed

+78
-77
lines changed

hardware-wallet-tests/multiple-hardware-wallets.ts renamed to hardware-wallet-tests/connect-multiple-hardware-wallets.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
createSequentialResult,
77
initLedgerChannel,
88
createTestInstructions,
9+
waitForZombieMessages,
910
} from './utils';
1011

1112
export const run = () => {
@@ -32,14 +33,15 @@ export const run = () => {
3233
},
3334
]);
3435

35-
return new Promise((resolve) => {
36+
return new Promise<void>((resolve) => {
3637
hardwareWalletConnectionChannel.onReceive(
3738
async (message: { path: string; deviceModel: string }) => {
3839
const [expectedValue, isOver] = getNextExpectedSequence();
3940
expect(message).toEqual(expectedValue);
4041

4142
if (isOver) {
42-
resolve(null);
43+
await waitForZombieMessages();
44+
resolve();
4345
}
4446
}
4547
);

hardware-wallet-tests/remove-multiple-hardware-wallets.ts renamed to hardware-wallet-tests/disconnect-multiple-hardware-wallets.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
createSequentialResult,
77
initLedgerChannel,
88
createTestInstructions,
9+
waitForZombieMessages,
910
} from './utils';
1011

1112
const getNextExpectedSequence = createSequentialResult([
@@ -41,15 +42,16 @@ export const run = () => {
4142
'Disconnect Nano X',
4243
]);
4344

44-
return new Promise((resolve) => {
45+
return new Promise<void>((resolve) => {
4546
hardwareWalletConnectionChannel.onReceive(
4647
async (params: { path: string; deviceModel: string }) => {
4748
const [expectedValue, isOver] = getNextExpectedSequence();
4849

4950
expect(params).toEqual(expectedValue);
5051

5152
if (isOver) {
52-
return resolve(null);
53+
await waitForZombieMessages();
54+
return resolve();
5355
}
5456
}
5557
);

hardware-wallet-tests/remove-single-hardware-wallet.ts renamed to hardware-wallet-tests/disconnect-single-hardware-wallet.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
initLedgerChannel,
77
createTestInstructions,
88
createSequentialResult,
9+
waitForZombieMessages,
910
} from './utils';
1011

1112
export const run = () => {
@@ -29,13 +30,16 @@ export const run = () => {
2930
},
3031
]);
3132

32-
return new Promise((resolve) => {
33+
return new Promise<void>((resolve) => {
3334
hardwareWalletConnectionChannel.onReceive(
3435
async (params: { path: string }) => {
3536
const [expectedValue, isOver] = getNextExpectedSequence();
3637
expect(params).toEqual(expectedValue);
3738

38-
if (isOver) return resolve(null);
39+
if (isOver) {
40+
await waitForZombieMessages();
41+
return resolve();
42+
}
3943
}
4044
);
4145

hardware-wallet-tests/index.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import prompts from 'prompts';
22

3-
import { run as runRemoveMultipleHardwareWallets } from './remove-multiple-hardware-wallets';
3+
import { run as runDisconnectMultipleHardwareWallets } from './disconnect-multiple-hardware-wallets';
44
import { run as runCardanoAppAlreadyLaunched } from './cardano-app-already-launched';
5-
import { run as runCardanoAppNotStarted } from './cardano-app-not-started';
6-
import { run as runRemoveSingleHardwareWallet } from './remove-single-hardware-wallet';
7-
import { run as runMultipleHardwareWallets } from './multiple-hardware-wallets';
5+
import { run as runCardanoAppNotLaunched } from './cardano-app-not-started';
6+
import { run as runDisconnectSingleHardwareWallet } from './disconnect-single-hardware-wallet';
7+
import { run as runConnectMultipleHardwareWallets } from './connect-multiple-hardware-wallets';
88

99
const CARDANO_APP_ALREADY_LAUNCHED = 'CARDANO_APP_ALREADY_LAUNCHED';
1010
const CARDANO_APP_NOT_STARTED = 'CARDANO_APP_NOT_STARTED';
@@ -49,19 +49,19 @@ const MULTIPLE_HARDWARE_WALLETS_REMOVED = 'MULTIPLE_HARDWARE_WALLETS_REMOVED';
4949
break;
5050

5151
case CARDANO_APP_NOT_STARTED:
52-
await runCardanoAppNotStarted();
52+
await runCardanoAppNotLaunched();
5353
break;
5454

5555
case SINGLE_LEDGER_DISCONNECTED:
56-
await runRemoveSingleHardwareWallet();
56+
await runDisconnectSingleHardwareWallet();
5757
break;
5858

5959
case MULTIPLE_HARDWARE_WALLETS:
60-
await runMultipleHardwareWallets();
60+
await runConnectMultipleHardwareWallets();
6161
break;
6262

6363
case MULTIPLE_HARDWARE_WALLETS_REMOVED:
64-
await runRemoveMultipleHardwareWallets();
64+
await runDisconnectMultipleHardwareWallets();
6565
break;
6666

6767
default:

hardware-wallet-tests/utils.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@ import {
1717
import { createChannels } from '../source/main/ipc/createHardwareWalletIPCChannels';
1818
import { handleHardwareWalletRequests } from '../source/main/ipc/getHardwareWalletChannel';
1919

20-
const { ipcMain, ipcRenderer } = createIPCMock();
21-
22-
export { ipcMain, ipcRenderer };
20+
export const { ipcMain, ipcRenderer } = createIPCMock();
2321

2422
export class MockIpcChannel<Incoming, Outgoing> extends IpcChannel<
2523
Incoming,
@@ -57,7 +55,7 @@ export class MockIpcChannel<Incoming, Outgoing> extends IpcChannel<
5755
}
5856

5957
export const createAndRegisterHardwareWalletChannels = () =>
60-
// @ts-ignore fix-me later
58+
// @ts-expect-error Argument of type 'ipcRenderer' is not assignable to parameter of type 'BrowserWindow'.
6159
handleHardwareWalletRequests(ipcRenderer, createChannels(MockIpcChannel));
6260

6361
export const initLedgerChannel = () => {
@@ -129,3 +127,6 @@ export const log = (message: string) =>
129127
export const createTestInstructions = (messages: string[]) => {
130128
messages.forEach((m, i) => log(`${i + 1} - ${m}`));
131129
};
130+
131+
export const waitForZombieMessages = () =>
132+
new Promise((resolve) => setTimeout(resolve, 1000));

source/main/ipc/createHardwareWalletIPCChannels.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ export interface HardwareWalletChannels {
100100
showAddressMainResponse
101101
>;
102102

103-
waitForLedgerDevicesChannel: IpcChannel<
103+
waitForLedgerDevicesToConnectChannel: IpcChannel<
104104
waitForLedgerDevicesRequest,
105105
waitForLedgerDevicesResponse
106106
>;
@@ -130,6 +130,6 @@ export const createChannels = (
130130
deriveXpubChannel: new Channel(DERIVE_XPUB_CHANNEL),
131131
deriveAddressChannel: new Channel(DERIVE_ADDRESS_CHANNEL),
132132
showAddressChannel: new Channel(SHOW_ADDRESS_CHANNEL),
133-
waitForLedgerDevicesChannel: new Channel(WAIT_FOR_LEDGER_DEVICES),
133+
waitForLedgerDevicesToConnectChannel: new Channel(WAIT_FOR_LEDGER_DEVICES),
134134
};
135135
};

source/main/ipc/getHardwareWalletChannel.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ export const handleHardwareWalletRequests = async (
158158
deriveXpubChannel,
159159
deriveAddressChannel,
160160
showAddressChannel,
161-
waitForLedgerDevicesChannel,
161+
waitForLedgerDevicesToConnectChannel,
162162
}: HardwareWalletChannels
163163
) => {
164164
let deviceConnection = null;
@@ -229,10 +229,10 @@ export const handleHardwareWalletRequests = async (
229229
});
230230
};
231231

232-
waitForLedgerDevicesChannel.onRequest(async () => {
233-
logger.info('[HW-DEBUG] waitForLedgerDevicesChannel::waiting');
232+
waitForLedgerDevicesToConnectChannel.onRequest(async () => {
233+
logger.info('[HW-DEBUG] waitForLedgerDevicesToConnectChannel::waiting');
234234
const { device, deviceModel } = await waitForDevice();
235-
logger.info('[HW-DEBUG] waitForLedgerDevicesChannel::found');
235+
logger.info('[HW-DEBUG] waitForLedgerDevicesToConnectChannel::found');
236236
return {
237237
disconnected: false,
238238
deviceType: 'ledger',

source/main/ipc/hardwareWallets/ledger/deviceDetection/deviceDetection.ts

Lines changed: 19 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,23 @@ export type DeviceDetectionPayload = {
1010
type: 'add' | 'remove';
1111
} & TrackedDevice;
1212

13+
const getDetector = () => {
14+
if (TransportNodeHid.isSupported()) {
15+
logger.info('[HW-DEBUG] Using usb-detection');
16+
17+
return useEventDrivenDetection;
18+
}
19+
logger.info('[HW-DEBUG] Using polling');
20+
21+
return usePollingDrivenDetection;
22+
};
23+
1324
export const deviceDetection = (
14-
onAdd: (arg0: DeviceDetectionPayload) => void,
15-
onRemove: (arg0: DeviceDetectionPayload) => void
25+
onAdd: (payload: DeviceDetectionPayload) => void,
26+
onRemove: (payload: DeviceDetectionPayload) => void
1627
) => {
28+
// detect existing connected devices without blocking the subscription registration
29+
// https://github.com/LedgerHQ/ledgerjs/blob/master/packages/hw-transport-node-hid-singleton/src/TransportNodeHid.ts#L56
1730
Promise.resolve(DeviceTracker.getDevices()).then((devices) => {
1831
// this needs to run asynchronously so the subscription is defined during this phase
1932
for (const device of devices) {
@@ -29,42 +42,22 @@ export const deviceDetection = (
2942
const handleOnRemove = (trackedDevice: TrackedDevice) =>
3043
onRemove({ type: 'remove', ...trackedDevice });
3144

32-
let detectDevices: Detector;
33-
34-
if (TransportNodeHid.isSupported()) {
35-
logger.info('[HW-DEBUG] Using usb-detection');
36-
37-
detectDevices = useEventDrivenDetection;
38-
} else {
39-
logger.info('[HW-DEBUG] Using polling');
40-
41-
detectDevices = usePollingDrivenDetection;
42-
}
45+
const detectDevices = getDetector();
4346

4447
detectDevices(handleOnAdd, handleOnRemove);
4548
};
4649

4750
export const waitForDevice = () => {
48-
return new Promise<TrackedDevice>(async (resolve) => {
49-
const currentDevices = await DeviceTracker.getDevices();
51+
return new Promise<TrackedDevice>((resolve) => {
52+
const currentDevices = DeviceTracker.getDevices();
5053

5154
for (const device of currentDevices) {
5255
return resolve(DeviceTracker.getTrackedDeviceByPath(device.path));
5356
}
5457

55-
let detectDevices: Detector;
58+
const detectDevices = getDetector();
5659
let unsubscribe: DectorUnsubscriber = null;
5760

58-
if (TransportNodeHid.isSupported()) {
59-
logger.info('[HW-DEBUG] Using usb-detection');
60-
61-
detectDevices = useEventDrivenDetection;
62-
} else {
63-
logger.info('[HW-DEBUG] Using polling');
64-
65-
detectDevices = usePollingDrivenDetection;
66-
}
67-
6861
const handleOnAdd = (trackedDevice: TrackedDevice) => {
6962
if (unsubscribe) {
7063
unsubscribe();

source/main/ipc/hardwareWallets/ledger/deviceDetection/deviceTracker.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export class DeviceTracker {
2626
return { device, deviceModel, descriptor } as TrackedDevice;
2727
}
2828

29-
static getDevices() {
29+
static getDevices(): (Device & { deviceName?: string })[] {
3030
return getDevices();
3131
}
3232

source/main/ipc/hardwareWallets/ledger/deviceDetection/eventDrivenDetection.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ const deviceToLog = ({ productId, locationId, deviceAddress }) =>
1010

1111
let isMonitoring = false;
1212

13+
const USB_EVENT_BUFFER_DELAY = 1500;
14+
1315
const monitorUSBDevices = () => {
1416
if (!isMonitoring) {
1517
isMonitoring = true;
@@ -55,7 +57,7 @@ export const detectDevices: Detector = (onAdd, onRemove) => {
5557
}
5658

5759
timeout = null;
58-
}, 1500);
60+
}, USB_EVENT_BUFFER_DELAY);
5961
}
6062
};
6163

0 commit comments

Comments
 (0)