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 branded types and fix release device #14440

Draft
wants to merge 2 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
4 changes: 3 additions & 1 deletion packages/connect/src/device/Device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,9 @@ export class Device extends TypedEmitter<DeviceEvents> {
// session was acquired by another instance. but another might not have power to release interface
// so it only notified about its session acquiral and the interrupted instance should cooperate
// and release device too.
this.transport.releaseDevice(this.originalDescriptor.path);
if (this.originalDescriptor.session) {
this.transport.releaseDevice(this.originalDescriptor.session);
}
}

async _runInner<X>(fn: (() => Promise<X>) | undefined, options: RunOptions): Promise<void> {
Expand Down
9 changes: 5 additions & 4 deletions packages/transport/src/api/abstract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {
Success,
Logger,
DescriptorApiLevel,
PathInternal,
} from '../types';
import { success, error, unknownError } from '../utils/result';

Expand Down Expand Up @@ -68,7 +69,7 @@ export abstract class AbstractApi extends TypedEmitter<{
* read from device on path
*/
abstract read(
path: string,
path: PathInternal,
signal?: AbortSignal,
): AsyncResultWithTypedError<
Buffer,
Expand All @@ -85,7 +86,7 @@ export abstract class AbstractApi extends TypedEmitter<{
* write to device on path
*/
abstract write(
path: string,
path: PathInternal,
buffers: Buffer,
signal?: AbortSignal,
): AsyncResultWithTypedError<
Expand All @@ -102,7 +103,7 @@ export abstract class AbstractApi extends TypedEmitter<{
* set device to the state when it is available to read/write
*/
abstract openDevice(
path: string,
path: PathInternal,
first: boolean,
signal?: AbortSignal,
): AsyncResultWithTypedError<
Expand All @@ -118,7 +119,7 @@ export abstract class AbstractApi extends TypedEmitter<{
* set device to the state when it is available to openDevice again
*/
abstract closeDevice(
path: string,
path: PathInternal,
): AsyncResultWithTypedError<
undefined,
| typeof ERRORS.DEVICE_NOT_FOUND
Expand Down
6 changes: 4 additions & 2 deletions packages/transport/src/api/udp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
AbstractApiConstructorParams,
DEVICE_TYPE,
} from './abstract';
import { DescriptorApiLevel } from '../types';
import { DescriptorApiLevel, PathInternal } from '../types';
import * as ERRORS from '../errors';

export class UdpApi extends AbstractApi {
Expand Down Expand Up @@ -161,7 +161,9 @@ export class UdpApi extends AbstractApi {

public async enumerate(signal?: AbortSignal) {
// in theory we could support multiple devices, but we don't yet
const paths = this.debugLink ? ['127.0.0.1:21325'] : ['127.0.0.1:21324'];
const paths = this.debugLink
? [PathInternal('127.0.0.1:21325')]
: [PathInternal('127.0.0.1:21324')];

try {
const enumerateResult = await Promise.all(
Expand Down
4 changes: 2 additions & 2 deletions packages/transport/src/api/usb.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createDeferred, createTimeoutPromise } from '@trezor/utils';

import { AbstractApi, AbstractApiConstructorParams, DEVICE_TYPE } from './abstract';
import { DescriptorApiLevel } from '../types';
import { DescriptorApiLevel, PathInternal } from '../types';
import {
CONFIGURATION_ID,
ENDPOINT_ID,
Expand Down Expand Up @@ -121,7 +121,7 @@ export class UsbApi extends AbstractApi {

private devicesToDescriptors(): DescriptorApiLevel[] {
return this.devices.map(d => ({
path: d.path,
path: PathInternal(d.path),
type: this.matchDeviceType(d.device),
product: d.device.productId,
vendor: d.device.vendorId,
Expand Down
20 changes: 12 additions & 8 deletions packages/transport/src/sessions/background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,14 @@ import type {
HandleMessageParams,
HandleMessageResponse,
} from './types';
import type { Descriptor, PathInternal, PathPublic, Success } from '../types';

import type { Descriptor, PathInternal, Success } from '../types';
import { PathPublic, Session } from '../types';
import * as ERRORS from '../errors';

function typedObjectKeys<T extends Record<any, any>>(obj: T): Array<keyof T> {
return Object.keys(obj) as Array<keyof T>;
}

type DescriptorsDict = Record<PathInternal, Descriptor>;

// in nodeusb, enumeration operation takes ~3 seconds
Expand Down Expand Up @@ -123,7 +127,7 @@ export class SessionsBackground extends TypedEmitter<{
* - caller informs about current descriptors
*/
private enumerateDone(payload: EnumerateDoneRequest) {
const disconnectedDevices = Object.keys(this.descriptors).filter(
const disconnectedDevices = typedObjectKeys(this.descriptors).filter(
pathInternal => !payload.descriptors.find(d => d.path === pathInternal),
);

Expand All @@ -134,7 +138,7 @@ export class SessionsBackground extends TypedEmitter<{

payload.descriptors.forEach(d => {
if (!this.pathInternalPathPublicMap[d.path]) {
this.pathInternalPathPublicMap[d.path] = `${(this.lastPathId += 1)}`;
this.pathInternalPathPublicMap[d.path] = PathPublic(`${(this.lastPathId += 1)}`);
}
if (!this.descriptors[d.path]) {
this.descriptors[d.path] = {
Expand Down Expand Up @@ -186,7 +190,7 @@ export class SessionsBackground extends TypedEmitter<{
const unconfirmedSessions: DescriptorsDict = JSON.parse(JSON.stringify(this.descriptors));

this.lastSessionId++;
unconfirmedSessions[pathInternal].session = `${this.lastSessionId}`;
unconfirmedSessions[pathInternal].session = Session(`${this.lastSessionId}`);

return this.success({
session: unconfirmedSessions[pathInternal].session,
Expand All @@ -206,7 +210,7 @@ export class SessionsBackground extends TypedEmitter<{
if (!pathInternal || !this.descriptors[pathInternal]) {
return this.error(ERRORS.DESCRIPTOR_NOT_FOUND);
}
this.descriptors[pathInternal].session = `${this.lastSessionId}`;
this.descriptors[pathInternal].session = Session(`${this.lastSessionId}`);

return Promise.resolve(
this.success({
Expand Down Expand Up @@ -241,7 +245,7 @@ export class SessionsBackground extends TypedEmitter<{
}

private getPathBySession({ session }: GetPathBySessionRequest) {
const path = Object.keys(this.descriptors).find(
const path = typedObjectKeys(this.descriptors).find(
pathKey => this.descriptors[pathKey]?.session === session,
);

Expand Down Expand Up @@ -309,7 +313,7 @@ export class SessionsBackground extends TypedEmitter<{
}

private getInternal(pathPublic: PathPublic): PathInternal | undefined {
return Object.keys(this.pathInternalPathPublicMap).find(
return typedObjectKeys(this.pathInternalPathPublicMap).find(
internal => this.pathInternalPathPublicMap[internal] === pathPublic,
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/transport/src/transports/abstract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ export abstract class AbstractTransport extends TransportEmitter {
* For transports with native access (webusb), this informs lower transport layer
* that device is not going to be used anymore
*/
abstract releaseDevice(path: string): AsyncResultWithTypedError<void, string>;
abstract releaseDevice(session: Session): AsyncResultWithTypedError<void, string>;

/**
* Encode data and write it to transport layer
Expand Down
16 changes: 13 additions & 3 deletions packages/transport/src/transports/abstractApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import { receiveAndParse } from '../utils/receive';
import { SessionsClient } from '../sessions/client';
import * as ERRORS from '../errors';
import { Session } from '../types';

interface ConstructorParams extends AbstractTransportParams {
api: AbstractApi;
Expand Down Expand Up @@ -156,7 +157,7 @@
return this.error({ error: releaseIntentResponse.error });
}

const releasePromise = this.releaseDevice(releaseIntentResponse.payload.path);
const releasePromise = this.releaseDevice(session);
if (onClose) return this.success(undefined);

await releasePromise;
Expand Down Expand Up @@ -315,8 +316,17 @@
);
}

releaseDevice(path: string) {
return this.api.closeDevice(path);
releaseDevice(session: Session) {
return this.sessionsClient
.getPathBySession({
session,
})
.then(response => {
if (response.success) {
return this.api.closeDevice(response.payload.path);
}
return this.success(undefined);

Check failure on line 328 in packages/transport/src/transports/abstractApi.ts

View workflow job for this annotation

GitHub Actions / Linting and formatting

Expected blank line before this statement
});
}

stop() {
Expand Down
16 changes: 12 additions & 4 deletions packages/transport/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,18 @@ import type { DEVICE_TYPE } from '../api/abstract';

export * from './apiCall';

export type Session = `${number}`;

export type PathInternal = string;
export type PathPublic = `${number}`;
export type Session = `${number}` & { __type: 'Session' };
export const Session = (input: `${number}`) => {
return `${input}` as Session;
};
export type PathInternal = string & { __type: 'PathInternal' };
export const PathInternal = (input: string) => {
return input as PathInternal;
};
export type PathPublic = `${number}` & { __type: 'PathPublic' };
export const PathPublic = (input: `${number}`) => {
return `${input}` as PathPublic;
};

export type DescriptorApiLevel = {
path: PathInternal;
Expand Down
45 changes: 32 additions & 13 deletions packages/transport/tests/abstractUsb.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { UsbApi } from '../src/api/usb';
import { SessionsClient } from '../src/sessions/client';
import { SessionsBackground } from '../src/sessions/background';
import * as messages from '@trezor/protobuf/messages.json';
import { PathPublic, Session } from '../src/types';

// create devices otherwise returned from navigator.usb.getDevices
const createMockedDevice = (optional = {}) => ({
Expand Down Expand Up @@ -183,7 +184,7 @@ describe('Usb', () => {
const spy = jest.fn();
transport.on('transport-update', spy);

transport.handleDescriptorsChange([{ path: '1', session: null, type: 1 }]);
transport.handleDescriptorsChange([{ path: PathPublic('1'), session: null, type: 1 }]);

expect(spy).toHaveBeenCalledWith([
{ type: 'connected', descriptor: { path: '1', session: null, type: 1 } },
Expand Down Expand Up @@ -230,7 +231,9 @@ describe('Usb', () => {

jest.runAllTimers();

const result = await transport.acquire({ input: { path: '1', previous: null } });
const result = await transport.acquire({
input: { path: PathPublic('1'), previous: null },
});
expect(result).toEqual({
success: true,
payload: '1',
Expand All @@ -254,10 +257,14 @@ describe('Usb', () => {
transport.listen();

// set some initial descriptors
const acquireCall = transport.acquire({ input: { path: '1', previous: null } });
const acquireCall = transport.acquire({
input: { path: PathPublic('1'), previous: null },
});

setTimeout(() => {
sessionsClient.emit('descriptors', [{ path: '321', session: '1', type: 1 }]);
sessionsClient.emit('descriptors', [
{ path: PathPublic('321'), session: Session('1'), type: 1 },
]);
}, 1);

const res = await acquireCall;
Expand All @@ -280,10 +287,12 @@ describe('Usb', () => {
transport.listen();

// set some initial descriptors
const acquireCall = transport.acquire({ input: { path: '1', previous: null } });
const acquireCall = transport.acquire({
input: { path: PathPublic('1'), previous: null },
});
setTimeout(() => {
sessionsClient.emit('descriptors', [
{ path: '1', session: '2', type: 1, product: 21441 },
{ path: PathPublic('1'), session: Session('2'), type: 1, product: 21441 },
]);
}, 1);

Expand All @@ -297,7 +306,7 @@ describe('Usb', () => {
const res = await transport.call({
name: 'GetAddress',
data: {},
session: '1',
session: Session('1'),
protocol: v1Protocol,
});
expect(res).toEqual({ success: false, error: 'device disconnected during action' });
Expand All @@ -307,7 +316,9 @@ describe('Usb', () => {
it('call - with valid and invalid message.', async () => {
const { transport, sessionsBackground } = await initTest();
await transport.enumerate();
const acquireRes = await transport.acquire({ input: { path: '1', previous: null } });
const acquireRes = await transport.acquire({
input: { path: PathPublic('1'), previous: null },
});
expect(acquireRes.success).toEqual(true);
if (!acquireRes.success) return;

Expand Down Expand Up @@ -350,7 +361,9 @@ describe('Usb', () => {
it('send and receive.', async () => {
const { transport, sessionsBackground } = await initTest();
await transport.enumerate();
const acquireRes = await transport.acquire({ input: { path: '1', previous: null } });
const acquireRes = await transport.acquire({
input: { path: PathPublic('1'), previous: null },
});
expect(acquireRes.success).toEqual(true);
if (!acquireRes.success) return;

Expand Down Expand Up @@ -386,7 +399,9 @@ describe('Usb', () => {
it('send protocol-v1 with custom chunkSize', async () => {
const { transport, testUsbApi, sessionsBackground } = await initTest();
await transport.enumerate();
const acquireRes = await transport.acquire({ input: { path: '1', previous: null } });
const acquireRes = await transport.acquire({
input: { path: PathPublic('1'), previous: null },
});
expect(acquireRes.success).toEqual(true);
if (!acquireRes.success) return;

Expand Down Expand Up @@ -424,7 +439,9 @@ describe('Usb', () => {
it('release', async () => {
const { transport, sessionsBackground } = await initTest();
await transport.enumerate();
const acquireRes = await transport.acquire({ input: { path: '1', previous: null } });
const acquireRes = await transport.acquire({
input: { path: PathPublic('1'), previous: null },
});
expect(acquireRes.success).toEqual(true);
if (!acquireRes.success) return;

Expand All @@ -433,7 +450,7 @@ describe('Usb', () => {
// doesn't really matter what what message we send
const res = await transport.release({
session: acquireRes.payload,
path: '123',
path: PathPublic('123'),
onClose: false,
});
expect(res).toEqual({
Expand All @@ -446,7 +463,9 @@ describe('Usb', () => {
it('call - with use abort', async () => {
const { transport, sessionsBackground } = await initTest();
await transport.enumerate();
const acquireRes = await transport.acquire({ input: { path: '1', previous: null } });
const acquireRes = await transport.acquire({
input: { path: PathPublic('1'), previous: null },
});
if (!acquireRes.success) return;

const abort = new AbortController();
Expand Down
Loading
Loading