Skip to content
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

Transport test update #16474

Draft
wants to merge 13 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/connect-popup/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ const handleMessageInIframeMode = (

if (disposed) return;

log.debug('handleMessage', data);
// log.debug('handleMessage', data);
if (data.type === RESPONSE_EVENT) {
handleResponseEvent(data);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/connect-web/src/impl/core-in-iframe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export class CoreInIframe implements ConnectFactoryDependencies<ConnectSettingsW

const message = parseMessage<CoreEventMessage>(messageEvent.data);

this._log.log('handleMessage', message);
// this._log.log('handleMessage', message);

switch (message.event) {
case RESPONSE_EVENT: {
Expand Down
84 changes: 84 additions & 0 deletions packages/connect/e2e/tests/device/deviceLifecycle.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
7 changes: 5 additions & 2 deletions packages/connect/src/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down Expand Up @@ -1085,7 +1088,7 @@ export class Core extends EventEmitter {
}

handleMessage(message: CoreRequestMessage) {
_log.debug('handleMessage', message);
// _log.debug('handleMessage', message);

switch (message.type) {
case POPUP.HANDSHAKE:
Expand Down
18 changes: 18 additions & 0 deletions packages/connect/src/core/onCallFirmwareUpdate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
14 changes: 10 additions & 4 deletions packages/connect/src/device/Device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,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
Expand Down Expand Up @@ -524,17 +526,21 @@ export class Device extends TypedEmitter<DeviceEvents> {
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
try {
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<typeof setTimeout> | undefined;

Expand Down
2 changes: 1 addition & 1 deletion packages/connect/src/device/DeviceCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 11 additions & 4 deletions packages/connect/src/device/__tests__/DeviceList.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
});
},
});
Expand Down
2 changes: 1 addition & 1 deletion packages/connect/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
2 changes: 1 addition & 1 deletion packages/suite-desktop-core/src/modules/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 6 additions & 1 deletion packages/suite-desktop-core/src/modules/trezor-connect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 13 additions & 0 deletions packages/suite-web/e2e/tests/wallet/discovery.test.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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}`);
Expand Down
16 changes: 15 additions & 1 deletion packages/suite/src/components/settings/DeviceBanner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -69,6 +70,19 @@ export const DeviceBanner = ({ title, description }: DeviceBannerProps) => {
</Row>
{description && <Text color="textSubdued">{description}</Text>}
</Column>

{/* todo: style, design, this was totally a blind shot */}
{device?.type === 'unacquired' && (
<Column>
<Button
onClick={() => {
TrezorConnect.getFeatures({ device });
}}
>
Use device here
</Button>
</Column>
)}
</Row>
</Card>
);
Expand Down
2 changes: 2 additions & 0 deletions packages/transport-bridge/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
17 changes: 12 additions & 5 deletions packages/transport/src/api/usb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand Down Expand Up @@ -493,21 +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_NO_DEVICE',
'LIBUSB_ERROR_OTHER',
// 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 });
}

Expand Down
2 changes: 1 addition & 1 deletion suite-common/connect-init/src/connectInitThunks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading