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

@trezor/transport refactor #6509

Merged
merged 1 commit into from
Jun 12, 2023
Merged

@trezor/transport refactor #6509

merged 1 commit into from
Jun 12, 2023

Conversation

mroz22
Copy link
Contributor

@mroz22 mroz22 commented Oct 10, 2022

Based on

Changes in @trezor/transport:

  • removed debug-link support all together. We have not used it since trezor-user-env came into service.
  • sessions management. I am adding a module that is called "sessions background" it is used to control access to usb/device and holds information about sessions. This is not new, previously it was implemented here. This module could be used in various contexts. At the moment, we only have SharedWorker.
  • created interfaces folder with lowest-possible-level unifying abstraction over native transport interfaces. so far only webusb.
  • added abstract transport class for API unification which itself inhertis from a strongly typed event emiter. no method from transport should throw error. each method should return either Success or Error
  • added abstract USB class, derived from abstract transport class. Used for code sharing between webusb and nodeusb. This is basically compilation of withSharedConnections.ts and webusb.ts. This is the place that implements low-level transport interface and sessions.
  • webusb now does not run iteration cycle in 500ms endless loop. The reasons are that it does not feel right if there are connection events available and also it probably would not work for nodeusb where enumaration cycle takes considerably longer than 500ms
  • all transport calls are now abortable
  • race conditions related to "cancel" method should now be fixed. Errors such as "Unexpected response, expected Address, received Featrues" should not appear anymore or at least should appear less often.

Related issues

QA testing tips:

In the first place, focus on regression in communication with device. Nodeusb feature is still hidden behind debug (and I am even considering pulling it out into separate PR based on this). Some tips what to test:

  • google-chrome http://localhost:8000/ http://localhost:8000/ open multiple tabs at once
  • debugging shared workers chrome://inspect/#workers
  • multiple devices connected
  • mad-reconnecting in the middle of communication with device
  • cancel message (pin, passphrase, etc)
  • all of above for both webusb and bridge
  • when using bridge transport, app should survive when computer goes to sleep

QA

@mroz22 mroz22 changed the title Transport refactor 2 transport rewire Oct 10, 2022
@mroz22 mroz22 changed the title transport rewire [wip] transport rewire Oct 10, 2022
@socket-security
Copy link

socket-security bot commented Oct 12, 2022

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

@mroz22
Copy link
Contributor Author

mroz22 commented Oct 12, 2022

Finally managed to get to the point where desktop app works with nodeusb transport and code is structured more or less in a good way. Of course it is still terribly messy and I needed to disable few features (sessions, unacquired devices etc) to make it work for now.

@mroz22 mroz22 force-pushed the transport-refactor-2 branch 2 times, most recently from c9f567d to 65143c4 Compare October 12, 2022 18:48
@mroz22 mroz22 changed the title [wip] transport rewire @trezor/transport nodeusb Oct 24, 2022
@mroz22 mroz22 force-pushed the transport-refactor-2 branch 3 times, most recently from fd5bc6e to 7314d6f Compare October 25, 2022 20:50
@mroz22 mroz22 force-pushed the transport-refactor-2 branch 2 times, most recently from 5e24ca4 to db073cc Compare October 27, 2022 18:11
@mroz22 mroz22 force-pushed the transport-refactor-2 branch 5 times, most recently from 64e3751 to d194d5d Compare November 7, 2022 14:32
@mroz22 mroz22 force-pushed the transport-refactor-2 branch 6 times, most recently from a3e7b59 to 7fb966c Compare November 14, 2022 10:12
@mroz22 mroz22 force-pushed the transport-refactor-2 branch 3 times, most recently from 246dfa8 to 7d38290 Compare November 20, 2022 12:10
@mroz22 mroz22 force-pushed the transport-refactor-2 branch 5 times, most recently from 1dc014f to ced36b6 Compare May 15, 2023 06:32
@mroz22 mroz22 force-pushed the transport-refactor-2 branch 7 times, most recently from de4630d to 7ee72bc Compare May 23, 2023 21:41
@hynek-jina hynek-jina linked an issue May 24, 2023 that may be closed by this pull request
@mroz22 mroz22 force-pushed the transport-refactor-2 branch 7 times, most recently from a817b6e to 312c7a0 Compare May 25, 2023 14:43
Copy link
Contributor

@marekrjpolak marekrjpolak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😰

@socket-security
Copy link

New and updated dependency changes detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives1 Size Publisher
@types/sharedworker 🆕 0.0.84 None +0 282 kB types

Footnotes

  1. https://docs.socket.dev

@mroz22 mroz22 force-pushed the transport-refactor-2 branch 2 times, most recently from 7fbba85 to e379924 Compare June 10, 2023 08:36
@mroz22 mroz22 merged commit f7b97fb into develop Jun 12, 2023
@mroz22 mroz22 deleted the transport-refactor-2 branch June 12, 2023 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
4 participants