[wip] misc

.github/workflows/test-misc.yml

@@ -71,7 +71,7 @@ jobs:
 
       - run: yarn install --immutable
       - run: yarn message-system-sign-config
-      - run: yarn test:unit
+      - run: yarn test:unit --detectOpenHandles --no-cache --coverage false
 
   utility-scripts:
     runs-on: ubuntu-latest

packages/connect-examples/electron-renderer-with-assets/webpack/config.dev.js

@@ -1,4 +1,4 @@
-const CopyPlugin = require('copy-webpack-plugin');
+const CopyWebpackPlugin = require('copy-webpack-plugin');
 const HtmlWebpackPlugin = require('html-webpack-plugin');
 const path = require('path');
 const { WebpackPluginServe } = require('webpack-plugin-serve');
@@ -28,7 +28,7 @@ module.exports = {
         ],
     },
     plugins: [
-        new CopyPlugin({
+        new CopyWebpackPlugin({
             patterns: [
                 {
                     from: path.join(__dirname, '..', '..', '..', 'connect-iframe', 'build'),

packages/connect-examples/electron-renderer-with-assets/webpack/config.prod.js

@@ -1,4 +1,4 @@
-const CopyPlugin = require('copy-webpack-plugin');
+const CopyWebpackPlugin = require('copy-webpack-plugin');
 const HtmlWebpackPlugin = require('html-webpack-plugin');
 const path = require('path');
 
@@ -25,7 +25,7 @@ module.exports = {
         modules: ['node_modules'],
     },
     plugins: [
-        new CopyPlugin({
+        new CopyWebpackPlugin({
             patterns: [
                 {
                     from: path.join(__dirname, '..', '..', '..', 'connect-iframe', 'build'),

packages/connect-explorer/src/actions/methodActions.ts

@@ -158,6 +158,10 @@ export const onCodeChange = (value: string) => (dispatch: Dispatch, getState: Ge
         };
         fields.forEach(processField);
     } catch (error) {
-        console.error('Invalid JSON', error);
+        if (error.message.includes('JSON5:')) {
+            console.warn('Invalid JSON', error);
+        } else {
+            console.error(error);
+        }
     }
 };

packages/connect-explorer/webpack/webextension.webpack.config.ts

@@ -1,4 +1,4 @@
-import CopyPlugin from 'copy-webpack-plugin';
+import CopyWebpackPlugin from 'copy-webpack-plugin';
 import HtmlWebpackPlugin from 'html-webpack-plugin';
 import path from 'path';
 import webpack from 'webpack';
@@ -111,7 +111,7 @@ const config: webpack.Configuration = {
             inject: true,
             minify: false,
         }),
-        new CopyPlugin({
+        new CopyWebpackPlugin({
             patterns: [
                 {
                     from: path.join(__dirname, '..', 'src-webextension', 'manifest.json'),

packages/connect-popup/webpack/prod.webpack.config.ts

@@ -1,5 +1,5 @@
 import { execSync } from 'child_process';
-import CopyPlugin from 'copy-webpack-plugin';
+import CopyWebpackPlugin from 'copy-webpack-plugin';
 import HtmlWebpackPlugin from 'html-webpack-plugin';
 import path from 'path';
 import TerserPlugin from 'terser-webpack-plugin';
@@ -107,7 +107,7 @@ const config: webpack.Configuration = {
             minify: false,
             urls: URLS,
         }),
-        new CopyPlugin({
+        new CopyWebpackPlugin({
             patterns: [
                 {
                     from: `${STATIC_SRC}/popup.css`,

packages/connect-web/src/module/index.ts

@@ -79,3 +79,7 @@ const TrezorConnect = factory(
 
 export default TrezorConnect;
 export * from '@trezor/connect/src/exports';
+
+window.addEventListener('beforeunload', () => {
+    impl.dispose();
+});

packages/connect-webextension/webpack/inline.webpack.config.ts

@@ -1,4 +1,4 @@
-import CopyPlugin from 'copy-webpack-plugin';
+import CopyWebpackPlugin from 'copy-webpack-plugin';
 import path from 'path';
 import webpack from 'webpack';
 
@@ -30,7 +30,7 @@ const config: webpack.Configuration = {
     module: prod.module,
     resolve: prod.resolve,
     plugins: [
-        new CopyPlugin({
+        new CopyWebpackPlugin({
             patterns: [
                 {
                     from: `${path.join(__dirname, '..', 'dist', 'content-script.js')}`,

packages/connect/e2e/tests/device/deviceLifecycle.test.ts

@@ -0,0 +1,84 @@
+import TrezorConnect from '../../../src';
+import { getController, initTrezorConnect, setup } 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);
+    });
+});

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 &&

packages/connect/src/device/Device.ts

@@ -363,6 +363,7 @@ export class Device extends TypedEmitter<DeviceEvents> {
                         error.message === TRANSPORT_ERROR.HTTP_ERROR // bridge died during device initialization
                     ) {
                         // disconnected, do nothing
+                        this.emitLifecycle(DEVICE.DISCONNECT);
                     } else if (
                         // we don't know what really happened
                         error.message === TRANSPORT_ERROR.UNEXPECTED_ERROR ||
@@ -374,6 +375,8 @@ export class Device extends TypedEmitter<DeviceEvents> {
                         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
@@ -565,6 +568,7 @@ export class Device extends TypedEmitter<DeviceEvents> {
 
                     // do not initialize while firstRunPromise otherwise `features.session_id` could be affected
                     await Promise.race([
+                        // todo: there is a issue at least with bridge (old and new) that getFeatures timeout does not kill the request and sending subsequent initialize gets 'other call in progress' error
                         this.getFeatures().finally(() => clearTimeout(getFeaturesTimeoutId)),
                         // note: tested on 24.7.2024 and whatever is written below this line is still valid
                         // We do not support T1B1 <1.9.0 but we still need Features even from not supported devices to determine your version
@@ -1170,12 +1174,13 @@ export class Device extends TypedEmitter<DeviceEvents> {
         return null;
     }
 
-    async dispose() {
+    dispose() {
         this.removeAllListeners();
         if (this.session && this.lastAcquiredHere) {
             try {
-                await this.cancelableAction?.();
-                await this.commands?.cancel();
+                // note for review: these deleted lines might not be needed anymore - in case of reload we don't have enough time
+                // anyway, so seems to be enough to release session so that we don't get 'unacquired' device after reloading. If
+                // anything stays on the screen, it will get flushed away by the initial Cancel (see device run method)
 
                 return this.transport.release({
                     session: this.session,

packages/connect/src/device/DeviceList.ts

@@ -400,23 +400,18 @@
     }
 
     private scheduleUpgradeCheck(apiType: TransportApiType, initParams: InitParams) {
-        clearTimeout(this.scheduledUpgradeChecks[apiType]);
-        this.scheduledUpgradeChecks[apiType] = setTimeout(async () => {
-            const transport = this.transport[apiType];
-            const transports = this.transports.filter(t => t.apiType === apiType);
-            if (!transport || transport === transports[0]) return;
-            for (const t of transports) {
-                if (t === transport) break;
-                if (await t.ping()) {
-                    this.transportLock(apiType, 'Upgrading', signal =>
-                        this.createInitPromise(apiType, initParams, signal),
-                    ).catch(() => {});
-
-                    return;
-                }
-            }
-            this.scheduleUpgradeCheck(apiType, initParams);
-        }, 1000);
+        this.scheduledUpgradeChecks[apiType];
+        const transport = this.transport[apiType];
+        const transports = this.transports.filter(t => t.apiType === apiType);
+        if (!transport || transport === transports[0]) return;
+        for (const t of transports) {
+            if (t === transport) break;
+            t.on('upgrade-available', () => {
+                this.transportLock(apiType, 'Upgrading', signal =>
+                    this.createInitPromise(apiType, initParams, signal),
+                ).catch(() => {});
+            });
+        }
     }
 
     private async selectTransport(
@@ -545,26 +540,27 @@
         return this.devices.all().find(d => d.features?.device_id === deviceId);
     }
 
-    async dispose() {
+    dispose() {
         this.removeAllListeners();
 
         const promises = typedObjectKeys(this.transport)
             .concat(typedObjectKeys(this.locks))
             .filter(arrayDistinct)
-            .map(apiType =>
+            .map(apiType => {
                 this.transportLock(apiType, 'Disposing', async () => {
                     const transport = this.transport[apiType];
+
                     if (transport) {
                         delete this.transport[apiType];
-                        await this.stopTransport(transport);
+                        this.stopTransport(transport);
                     }
-                }),
-            );
+                });
+            });
 
-        await Promise.all(promises);
+        Promise.all(promises);
     }
 
-    private async stopTransport(transport: Transport) {
+    private stopTransport(transport: Transport) {
         clearTimeout(this.scheduledUpgradeChecks[transport.apiType]);
 
         const devices = this.devices.clear(transport);
@@ -576,16 +572,16 @@
         });
 
         // release all devices
-        await Promise.all(
+        return Promise.all(
             devices.map(async device => {
                 this.authPenaltyManager.remove(device); // TODO is this right?
-                await device.dispose();
+                return device.dispose();
             }),
-        );
-
-        // now we can be relatively sure that release calls have been dispatched
-        // and we can safely kill all async subscriptions in transport layer
-        transport?.stop();
+        ).finally(() => {
+            // now we can be relatively sure that release calls have been dispatched
+            // and we can safely kill all async subscriptions in transport layer
+            transport?.stop();
+        });
     }
 
     async enumerate() {

packages/connect/src/device/__tests__/DeviceList.test.ts

@@ -141,20 +141,14 @@ describe('DeviceList', () => {
         list.init({ transports: [transport], pendingTransportEvent: true });
         const transportFirstEvent = list.pendingConnection();
 
-        // NOTE: this behavior is wrong, if device creation fails DeviceList shouldn't wait 10 secs.
-        jest.useFakeTimers();
-        // move 9 sec forward
-        await jest.advanceTimersByTimeAsync(9 * 1000);
         // no events yet
         expect(eventsSpy).toHaveBeenCalledTimes(0);
-        // move 2 sec forward
-        await jest.advanceTimersByTimeAsync(2 * 1000);
-        // promise should be resolved by now
         await transportFirstEvent;
-        jest.useRealTimers();
+        const events = eventsSpy.mock.calls.map(call => call[0]);
+        expect(events).toEqual(['device-disconnect', 'transport-start']);
 
-        expect(eventsSpy).toHaveBeenCalledTimes(1);
-        expect(eventsSpy.mock.calls[0][0]).toEqual('transport-start');
+        // todo: device should also disappear from DeviceList but it does not. this needs to get discussed
+        expect(list.getDeviceCount()).toEqual(0);
     });
 
     it('.init() with pendingTransportEvent (unreadable device)', async () => {

packages/suite-build/configs/desktop.webpack.config.ts

@@ -1,4 +1,4 @@
-import CopyPlugin from 'copy-webpack-plugin';
+import CopyWebpackPlugin from 'copy-webpack-plugin';
 import HtmlWebpackPlugin from 'html-webpack-plugin';
 import path from 'path';
 import webpack from 'webpack';
@@ -25,7 +25,7 @@ const config: webpack.Configuration = {
         path: path.join(baseDir, 'build'),
     },
     plugins: [
-        new CopyPlugin({
+        new CopyWebpackPlugin({
             patterns: ['bin', 'fonts', 'images', 'videos', 'guide/assets']
                 .map(dir => ({
                     from: path.join(__dirname, '..', '..', 'suite-data', 'files', dir),