From 03a855f2a916dc2374f2b83fc38fd451c6511087 Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Thu, 23 Jan 2025 15:33:57 +0100 Subject: [PATCH 01/13] chore(transport-test): add a todo note --- packages/transport-bridge/src/http.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/transport-bridge/src/http.ts b/packages/transport-bridge/src/http.ts index 28b66e3c360..7101a2157de 100644 --- a/packages/transport-bridge/src/http.ts +++ b/packages/transport-bridge/src/http.ts @@ -178,6 +178,8 @@ export class TrezordNode { }; res.addListener('close', listener); + // todo: is listener removed correctly? I noted it was firing long after the request was settled. + return abortController.signal; } From eb4662e71088dd2d9d414835c740338ba7e005a8 Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Thu, 23 Jan 2025 17:38:12 +0100 Subject: [PATCH 02/13] feat(suite-desktop): enable node-bridge in EAP --- packages/suite-desktop-core/src/modules/bridge.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/suite-desktop-core/src/modules/bridge.ts b/packages/suite-desktop-core/src/modules/bridge.ts index e1910bde571..3380514d8db 100644 --- a/packages/suite-desktop-core/src/modules/bridge.ts +++ b/packages/suite-desktop-core/src/modules/bridge.ts @@ -91,7 +91,7 @@ const shouldUseLegacyBridge = (store: Dependencies['store']) => { // dev uses node-bridge by default if (isDevEnv) return false; // eap has node-bridge temporarily disabled - if (allowPrerelease) return true; + if (allowPrerelease) return false; // handle rollout for regular users if (skipNewBridgeRollout) return false; From 4677b0fc801ad12a800d65a339a7fd25302f02a5 Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Thu, 23 Jan 2025 18:51:17 +0100 Subject: [PATCH 03/13] tmp: logs --- packages/connect-popup/src/index.tsx | 2 +- packages/connect-web/src/impl/core-in-iframe.ts | 2 +- packages/connect/src/core/index.ts | 2 +- packages/connect/src/device/DeviceCommands.ts | 2 +- packages/connect/src/index.ts | 2 +- suite-common/connect-init/src/connectInitThunks.ts | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/connect-popup/src/index.tsx b/packages/connect-popup/src/index.tsx index 45f59a064d2..8df827897d5 100644 --- a/packages/connect-popup/src/index.tsx +++ b/packages/connect-popup/src/index.tsx @@ -242,7 +242,7 @@ const handleMessageInIframeMode = ( if (disposed) return; - log.debug('handleMessage', data); + // log.debug('handleMessage', data); if (data.type === RESPONSE_EVENT) { handleResponseEvent(data); } diff --git a/packages/connect-web/src/impl/core-in-iframe.ts b/packages/connect-web/src/impl/core-in-iframe.ts index 79288ddd740..05ee490385d 100644 --- a/packages/connect-web/src/impl/core-in-iframe.ts +++ b/packages/connect-web/src/impl/core-in-iframe.ts @@ -103,7 +103,7 @@ export class CoreInIframe implements ConnectFactoryDependencies(messageEvent.data); - this._log.log('handleMessage', message); + // this._log.log('handleMessage', message); switch (message.event) { case RESPONSE_EVENT: { diff --git a/packages/connect/src/core/index.ts b/packages/connect/src/core/index.ts index a701a92c7cc..cd3a2532e43 100644 --- a/packages/connect/src/core/index.ts +++ b/packages/connect/src/core/index.ts @@ -1085,7 +1085,7 @@ export class Core extends EventEmitter { } handleMessage(message: CoreRequestMessage) { - _log.debug('handleMessage', message); + // _log.debug('handleMessage', message); switch (message.type) { case POPUP.HANDSHAKE: diff --git a/packages/connect/src/device/DeviceCommands.ts b/packages/connect/src/device/DeviceCommands.ts index 12170044b90..f26ba31fc7c 100644 --- a/packages/connect/src/device/DeviceCommands.ts +++ b/packages/connect/src/device/DeviceCommands.ts @@ -319,7 +319,7 @@ export class DeviceCommands { logger.debug( 'Received', res.payload.type, - filterForLog(res.payload.type, res.payload.message), + // filterForLog(res.payload.type, res.payload.message), ); // TODO: https://github.com/trezor/trezor-suite/issues/5301 diff --git a/packages/connect/src/index.ts b/packages/connect/src/index.ts index 116a38797cc..e9271d40f8a 100644 --- a/packages/connect/src/index.ts +++ b/packages/connect/src/index.ts @@ -58,7 +58,7 @@ const onCoreEvent = (message: CoreEventMessage) => { if (type === POPUP.CANCEL_POPUP_REQUEST) return; - _log.debug('handleMessage', message); + // _log.debug('handleMessage', message); switch (event) { case RESPONSE_EVENT: { diff --git a/suite-common/connect-init/src/connectInitThunks.ts b/suite-common/connect-init/src/connectInitThunks.ts index 6a06b0b854e..e6f532b3a84 100644 --- a/suite-common/connect-init/src/connectInitThunks.ts +++ b/suite-common/connect-init/src/connectInitThunks.ts @@ -157,7 +157,7 @@ export const connectInitThunk = createThunk( pendingTransportEvent: selectIsPendingTransportEvent(getState()), transports: selectDebugSettings(getState()).transports, _sessionsBackgroundUrl, - // debug: true, // Enable debug logs in TrezorConnect + debug: true, // Enable debug logs in TrezorConnect }); } catch (error) { let formattedError: string; From 8063ebbbd8ec11049d78f205e00d2335b6357444 Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Fri, 24 Jan 2025 11:37:09 +0100 Subject: [PATCH 04/13] fix(usb): timeoutable usb write --- packages/transport/src/api/usb.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/transport/src/api/usb.ts b/packages/transport/src/api/usb.ts index 0ee2cd09eee..8bc09bf1401 100644 --- a/packages/transport/src/api/usb.ts +++ b/packages/transport/src/api/usb.ts @@ -233,14 +233,21 @@ export class UsbApi extends AbstractApi { try { // https://wicg.github.io/webusb/#ref-for-dom-usbdevice-transferout this.logger?.debug('usb: device.transferOut'); + + const timeout = setTimeout(() => { + device?.reset(); + }, 1000); + const result = await this.abortableMethod( () => device.transferOut( this.debugLink ? DEBUGLINK_ENDPOINT_ID : ENDPOINT_ID, newArray, ), + { signal, onAbort: () => device?.reset() }, ); + clearTimeout(timeout); this.logger?.debug(`usb: device.transferOut done.`); if (result.status !== 'ok') { this.logger?.error(`usb: device.transferOut status not ok: ${result.status}`); @@ -498,11 +505,12 @@ export class UsbApi extends AbstractApi { if ( [ // node usb - 'LIBUSB_TRANSFER_ERROR', - 'LIBUSB_ERROR_PIPE', - 'LIBUSB_ERROR_IO', + // 'LIBUSB_TRANSFER_ERROR', + // 'LIBUSB_ERROR_PIPE', + // 'LIBUSB_ERROR_IO', + // 'LIBUSB_ERROR_OTHER', + 'LIBUSB_ERROR_NO_DEVICE', - 'LIBUSB_ERROR_OTHER', // web usb ERRORS.INTERFACE_DATA_TRANSFER, 'The device was disconnected.', From a16bbb921712b1902bab1737e3c18588f5465e72 Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Fri, 24 Jan 2025 11:42:58 +0100 Subject: [PATCH 05/13] fix(transport): rework usb disconnected error handling --- packages/transport/src/api/usb.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/transport/src/api/usb.ts b/packages/transport/src/api/usb.ts index 8bc09bf1401..c24196754a1 100644 --- a/packages/transport/src/api/usb.ts +++ b/packages/transport/src/api/usb.ts @@ -500,22 +500,21 @@ export class UsbApi extends AbstractApi { return [hidDevices, nonHidDevices]; } + // old bridge narrows multiple different errors to "device was disconnected" error. // https://github.com/trezor/trezord-go/blob/db03d99230f5b609a354e3586f1dfc0ad6da16f7/usb/libusb.go#L545 + // I am not sure if this is correct so at this point we only narrow disconnected error from node-usb and webusb private handleReadWriteError(err: Error) { if ( [ // node usb - // 'LIBUSB_TRANSFER_ERROR', - // 'LIBUSB_ERROR_PIPE', - // 'LIBUSB_ERROR_IO', - // 'LIBUSB_ERROR_OTHER', - 'LIBUSB_ERROR_NO_DEVICE', // web usb - ERRORS.INTERFACE_DATA_TRANSFER, 'The device was disconnected.', ].some(disconnectedErr => err.message.includes(disconnectedErr)) ) { + // make sure that local descriptors are updated and higher layers are notified + this.enumerate(); + return this.error({ error: ERRORS.DEVICE_DISCONNECTED_DURING_ACTION }); } From 4216920c7fa575f7e0df723bfb5d1672d0bf6efa Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Fri, 24 Jan 2025 11:43:41 +0100 Subject: [PATCH 06/13] fix(connect): flush device with Cancel command on first interaction to prevent unexpected messages being read --- packages/connect/src/device/Device.ts | 12 ++++++++---- .../src/device/__tests__/DeviceList.test.ts | 15 +++++++++++---- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/connect/src/device/Device.ts b/packages/connect/src/device/Device.ts index 1dd3b24ad62..9af0fb7a65f 100644 --- a/packages/connect/src/device/Device.ts +++ b/packages/connect/src/device/Device.ts @@ -524,6 +524,11 @@ export class Device extends TypedEmitter { await this.acquire(); } + const getFeaturesTimeout = + DataManager.getSettings('env') === 'react-native' + ? GET_FEATURES_TIMEOUT_REACT_NATIVE + : GET_FEATURES_TIMEOUT; + const { staticSessionId, deriveCardano } = this.getState() || {}; if (acquireNeeded || !staticSessionId || (!deriveCardano && options.useCardanoDerivation)) { // update features @@ -531,10 +536,9 @@ export class Device extends TypedEmitter { if (fn) { await this.initialize(!!options.useCardanoDerivation); } else { - const getFeaturesTimeout = - DataManager.getSettings('env') === 'react-native' - ? GET_FEATURES_TIMEOUT_REACT_NATIVE - : GET_FEATURES_TIMEOUT; + _log.debug('sending cancel'); + await cancelPrompt(this, true).catch(() => {}); + _log.debug('sending cancel done'); let getFeaturesTimeoutId: ReturnType | undefined; diff --git a/packages/connect/src/device/__tests__/DeviceList.test.ts b/packages/connect/src/device/__tests__/DeviceList.test.ts index 2b5d83a08d6..613f339343f 100644 --- a/packages/connect/src/device/__tests__/DeviceList.test.ts +++ b/packages/connect/src/device/__tests__/DeviceList.test.ts @@ -280,15 +280,22 @@ describe('DeviceList', () => { let readCount = 0; const transport = createTestTransport({ read: () => { - let features = '3f232300110000000c1002180020006000aa010154'; // 2.0.0 - if (readCount > 0) { - features = `3f232300110000000c10021800200${1}6000aa010154`; // 2.0.1 + let res = ''; + if (readCount === 0) { + // cancel response + res = '3f2323000300000000000000000000000000000000'; + } else if (readCount === 1) { + // features + res = '3f232300110000000c1002180020006000aa010154'; // 2.0.0; + } else { + // features + res = `3f232300110000000c10021800200${1}6000aa010154`; // 2.0.1 } readCount++; return Promise.resolve({ success: true, - payload: Buffer.from(features, 'hex'), + payload: Buffer.from(res, 'hex'), }); }, }); From d94cc26f40f5518406eda565e2133c94c7cd8dab Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Mon, 27 Jan 2025 12:01:03 +0100 Subject: [PATCH 07/13] test(suite-web): test app reload during discovery --- .../suite-web/e2e/tests/wallet/discovery.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/suite-web/e2e/tests/wallet/discovery.test.ts b/packages/suite-web/e2e/tests/wallet/discovery.test.ts index b260558bfdc..23ae7e0d909 100644 --- a/packages/suite-web/e2e/tests/wallet/discovery.test.ts +++ b/packages/suite-web/e2e/tests/wallet/discovery.test.ts @@ -1,6 +1,8 @@ // @group_wallet // @retry=2 +import { getRandomInt } from '@trezor/utils'; + import { onNavBar } from '../../support/pageObjects/topBarObject'; // discovery should end within this time frame @@ -43,6 +45,17 @@ describe('Discovery', () => { cy.log('all available networks should return something from discovery'); cy.getTestElement('@dashboard/loading', { timeout: 1000 * 10 }); + + // wait randomly between 1000 and 5000 ms + cy.wait(getRandomInt(1, 6) * 1000); + // trigger reload to simulate interruption + cy.reload(); + + // device appears as connected + cy.getTestElement('@deviceStatus-connected'); + // dashboard is still loading, discovery starts, no error appears + cy.getTestElement('@dashboard/loading'); + cy.getTestElement('@dashboard/loading', { timeout: DISCOVERY_LIMIT }).should('not.exist'); ['btc', ...coinsToActivate].forEach(symbol => { cy.getTestElement(`@wallet/coin-balance/value-${symbol}`); From 60bdecd9520be313f7c2b713338639712b0298eb Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Mon, 27 Jan 2025 12:55:16 +0100 Subject: [PATCH 08/13] chore(suite-desktop): enable trezor-connect logger when debug mode is active --- packages/suite-desktop-core/src/modules/trezor-connect.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/suite-desktop-core/src/modules/trezor-connect.ts b/packages/suite-desktop-core/src/modules/trezor-connect.ts index 9bc130cda53..a36a3b713b6 100644 --- a/packages/suite-desktop-core/src/modules/trezor-connect.ts +++ b/packages/suite-desktop-core/src/modules/trezor-connect.ts @@ -56,7 +56,12 @@ export const initBackground: ModuleInitBackground = ({ mainThreadEmitter, store onRequest: async (method, params) => { logger.debug(SERVICE_NAME, `call ${method}`); if (method === 'init') { - const response = await TrezorConnect[method](...params); + const response = await TrezorConnect.init({ + ...params[0], + // poor mans solution to enable debug logs in trezor-connect. if debug mode is enabled in trezor-suite when application starts, we get also logs from connect + // todo: ideally connect should accept logger as a param + debug: logger.level === 'debug', + }); await setProxy(); return response; From 89626ec1ea75f473c505a1fa11d70191b48bc137 Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Mon, 27 Jan 2025 13:16:47 +0100 Subject: [PATCH 09/13] tmp: log possible issue --- packages/connect/src/device/Device.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/connect/src/device/Device.ts b/packages/connect/src/device/Device.ts index 9af0fb7a65f..d46b71d07ba 100644 --- a/packages/connect/src/device/Device.ts +++ b/packages/connect/src/device/Device.ts @@ -368,6 +368,8 @@ export class Device extends TypedEmitter { error.code === 'Device_InitializeFailed' ) { this.emitLifecycle(DEVICE.CONNECT_UNACQUIRED); + // todo: shouldn't we enumerate here to prevent the case where backend has a different session from the local one and acquiring + // keeps returning "wrong previous session"? } else if ( // device was claimed by another application on transport api layer (claimInterface in usb nomenclature) but never released (releaseInterface in usb nomenclature) // the only remedy for this is to reconnect device manually From 452cbb138600f58f2b46e63769f3e0821914a2a7 Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Mon, 27 Jan 2025 13:29:26 +0100 Subject: [PATCH 10/13] fix(suite): add missing 'use device here' button in the device settings --- .../src/components/settings/DeviceBanner.tsx | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/suite/src/components/settings/DeviceBanner.tsx b/packages/suite/src/components/settings/DeviceBanner.tsx index 6621bb0e81f..d5e6a1d1c9c 100644 --- a/packages/suite/src/components/settings/DeviceBanner.tsx +++ b/packages/suite/src/components/settings/DeviceBanner.tsx @@ -2,7 +2,8 @@ import { ReactNode } from 'react'; import styled from 'styled-components'; -import { Card, LottieAnimation, Paragraph, Row, variables, Text } from '@trezor/components'; +import TrezorConnect from '@trezor/connect'; +import { Card, LottieAnimation, Paragraph, Row, variables, Text, Button } from '@trezor/components'; import { spacings } from '@trezor/theme'; import { useDevice, useSelector } from 'src/hooks/suite'; @@ -69,6 +70,19 @@ export const DeviceBanner = ({ title, description }: DeviceBannerProps) => { {description && {description}} + + {/* todo: style, design, this was totally a blind shot */} + {device?.type === 'unacquired' && ( + + + + )} ); From 1189ae1d3572f67340e6a41751c690b77c9c9555 Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Mon, 27 Jan 2025 14:16:44 +0100 Subject: [PATCH 11/13] fix(connect): edgecase where device is not detected during fw update using the old bridge --- .../connect/src/core/onCallFirmwareUpdate.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/connect/src/core/onCallFirmwareUpdate.ts b/packages/connect/src/core/onCallFirmwareUpdate.ts index 117260ff4d6..d26334eeab0 100644 --- a/packages/connect/src/core/onCallFirmwareUpdate.ts +++ b/packages/connect/src/core/onCallFirmwareUpdate.ts @@ -96,6 +96,24 @@ const waitForReconnectedDevice = async ( } catch { /* empty */ } + + // general logic (DeviceList/Device) refuses to call getFeatures if the reported descriptor has a session. + // the reason for session to be still there is this scenario: + // 1. reboot to bootloader is called + // 2. old bridge uses cca 200ms enumeration loop. If device appears on usb in the right time, bridge does not consider it + // a disconnect and it does not flush sessions + // 3. listen now reported a new device in bootloader mode but it still has the session from the previous device in normal mode + // 4. now we automatically take the device, as if user clicked on the "use device here button" + if (!reconnectedDevice?.features) { + log.debug( + 'onCallFirmwareUpdate', + 'we were unable to read device.features on the first interaction after seeing it, retrying...', + ); + await reconnectedDevice?.run().catch(() => { + // empty, try in the next loop + }); + } + i++; log.debug('onCallFirmwareUpdate', 'waiting for device to reconnect', i); } while ( From 443d38c9c61c488ddf444612b15be5cbcaba5668 Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Mon, 27 Jan 2025 17:37:39 +0100 Subject: [PATCH 12/13] todo: device disconnected and release note --- packages/connect/src/core/index.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/connect/src/core/index.ts b/packages/connect/src/core/index.ts index cd3a2532e43..1ef820ab2ca 100644 --- a/packages/connect/src/core/index.ts +++ b/packages/connect/src/core/index.ts @@ -715,8 +715,11 @@ const onCallDevice = async ( // TODO: This requires a massive refactoring https://github.com/trezor/trezor-suite/issues/5323 // @ts-expect-error TODO: messageResponse should be assigned from the response of "inner" function const response = messageResponse; - if (response) { + // todo: shouldReleaseSession should probably be removed (it was added recently). it looks like that we could + // receive 'disconnected during action' which does not mean that device got physically disconnected. + // see + // https://github.com/trezor/trezord-go/blob/db03d99230f5b609a354e3586f1dfc0ad6da16f7/usb/libusb.go#L545 const shouldReleaseSession = response.success || (!response.success && From 320e91fcac384f88410cd7554d57dd3f5e0d4944 Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Tue, 28 Jan 2025 15:27:01 +0100 Subject: [PATCH 13/13] test(connect): [wip] device lifecycle tests --- .../e2e/tests/device/deviceLifecycle.test.ts | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 packages/connect/e2e/tests/device/deviceLifecycle.test.ts diff --git a/packages/connect/e2e/tests/device/deviceLifecycle.test.ts b/packages/connect/e2e/tests/device/deviceLifecycle.test.ts new file mode 100644 index 00000000000..5d80b0def75 --- /dev/null +++ b/packages/connect/e2e/tests/device/deviceLifecycle.test.ts @@ -0,0 +1,84 @@ +import TrezorConnect from '../../../src'; +import { getController, setup, initTrezorConnect } from '../../common.setup'; + +const controller = getController(); + +describe('TrezorConnect device lifecycle tests', () => { + beforeAll(async () => { + await controller.connect(); + }); + afterAll(() => { + controller.dispose(); + TrezorConnect.dispose(); + }); + + it('TrezorConnect.init -> call', async () => { + TrezorConnect.dispose(); + TrezorConnect.removeAllListeners(); + await controller.stopEmu(); + await controller.startBridge(); + await initTrezorConnect(controller, { autoConfirm: false }); + + const firstDeviceEventSpy = jest.fn(); + TrezorConnect.on('device-connect', firstDeviceEventSpy); + const selectDeviceEventPromise = new Promise(resolve => { + TrezorConnect.on('ui-select_device', resolve); + }); + + TrezorConnect.getAddress({ + path: "m/44'/1'/0'/0/0", + showOnTrezor: false, + }); + + expect(await selectDeviceEventPromise).toMatchObject({ webusb: false, devices: [] }); + expect(firstDeviceEventSpy).toHaveBeenCalledTimes(0); + }); + + [1, 100, 1000].forEach(delay => { + it(`TrezorConnect.init -> startEmu -> wait ${delay}ms -> stopEmu -> startEmu -> device-connect event`, async () => { + TrezorConnect.dispose(); + TrezorConnect.removeAllListeners(); + await setup(controller, { + mnemonic: 'mnemonic_all', + }); + await controller.stopEmu(); + await initTrezorConnect(controller, { autoConfirm: false }); + + const deviceConnectEventPromise = new Promise(resolve => { + TrezorConnect.on('device-connect', resolve); + }); + + await controller.startEmu(); + + // testing disconnecting device during the initial reading of the device + await new Promise(resolve => setTimeout(resolve, delay)); + await controller.stopEmu(); + await controller.startEmu(); + + await deviceConnectEventPromise; + }); + }); + + it('TrezorConnect.init -> start emu -> device-connect event -> call', async () => { + TrezorConnect.dispose(); + TrezorConnect.removeAllListeners(); + await setup(controller, { + mnemonic: 'mnemonic_all', + }); + await controller.stopEmu(); + await initTrezorConnect(controller, { autoConfirm: false }); + + const deviceConnectEventPromise = new Promise(resolve => { + TrezorConnect.on('device-connect', resolve); + }); + + await controller.startEmu(); + await deviceConnectEventPromise; + + const response = await TrezorConnect.getAddress({ + path: "m/44'/1'/0'/0/0", + showOnTrezor: false, + }); + expect(response.success).toBe(true); + }); +});