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

chore: update Trezor Connect to v9.4.0, remove workarounds #27112

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

vthomas13
Copy link
Contributor

@vthomas13 vthomas13 commented Sep 12, 2024

This PR includes changes from PR #26749 by @martykan.

Description

With MV3, MetaMask started to use Trezor Connect inside an offscreen environment, in a way that was previously unsupported and required a workaround by patching the Trezor Connect library.

In recent versions of Trezor Connect, the library can handle working in an offscreen environment correctly, without the need for the workaround, which could cause issues with compatibility.

This PR removes the patch and updates the Trezor Connect library to the latest version (v9.4.0).
The change in manifest.json is due to new URL parameters, Firefox needs the asterisk at the end to match the URL with them.
I haven't removed the WebUSB device request which was added on MetaMask's side in relation to the workaround, since it can improve UX of the pairing process, but it could be removed if desired.

The dependency update affects Lavamoat, I am including the policy changes in my commit, however let me know if you would like me to remove them and handle them using your own process.

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Open accounts dropdown, "Add hardware wallet"
  2. Select Trezor
  3. Follow prompts to connect the device
  4. See a list of accounts from the Trezor

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@vthomas13 vthomas13 requested review from a team as code owners September 12, 2024 18:14
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

Copy link

socket-security bot commented Sep 12, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@babel/[email protected] None 0 489 kB nicolo-ribaudo
npm/@babel/[email protected] None 0 52.4 kB nicolo-ribaudo
npm/@babel/[email protected] None +1 1.23 MB nicolo-ribaudo
npm/@babel/[email protected] None 0 63.7 kB nicolo-ribaudo
npm/@babel/[email protected] None 0 160 kB nicolo-ribaudo
npm/@babel/[email protected] None 0 114 kB nicolo-ribaudo
npm/@babel/[email protected] None 0 106 kB nicolo-ribaudo
npm/@babel/[email protected] None 0 14.1 kB nicolo-ribaudo
npm/@babel/[email protected] None 0 58.5 kB nicolo-ribaudo
npm/@babel/[email protected] None 0 11.8 kB nicolo-ribaudo
npm/@babel/[email protected] None 0 1.89 MB nicolo-ribaudo
npm/@babel/[email protected] None 0 70 kB nicolo-ribaudo
npm/@babel/[email protected] None 0 69.7 kB existentialism, hzoo, jlhwung, ...1 more
npm/@babel/[email protected] None 0 107 kB nicolo-ribaudo
npm/@babel/[email protected] None 0 200 kB nicolo-ribaudo
npm/@babel/[email protected] None 0 90.8 kB nicolo-ribaudo
npm/@babel/[email protected] None 0 248 kB nicolo-ribaudo
npm/@babel/[email protected] None 0 70.4 kB nicolo-ribaudo
npm/@babel/[email protected] environment 0 2.48 MB nicolo-ribaudo
npm/@emurgo/[email protected] eval 0 3.35 MB lisicky_emurgo
npm/@emurgo/[email protected] eval, filesystem 0 3.35 MB lisicky_emurgo
npm/@fivebinaries/[email protected] None 0 149 kB slowbackspace
npm/@mobily/[email protected] None 0 381 kB mobily
npm/@sinclair/[email protected] None 0 536 kB sinclair
npm/@solana/[email protected] None 0 197 kB steveluscher
npm/@solana/[email protected] network +1 11 MB lorisleiva
npm/@trezor/[email protected] network 0 14 kB trezor-ci
npm/@trezor/[email protected] None 0 56.3 kB trezor-ci
npm/@trezor/[email protected] network 0 58.4 kB trezor-ci
npm/@trezor/[email protected] Transitive: environment, network +2 378 kB trezor-ci
npm/@trezor/[email protected] environment 0 5.41 kB trezor-ci
npm/@trezor/[email protected] None 0 180 kB trezor-ci
npm/@trezor/[email protected] None 0 93.8 kB trezor-ci
npm/@trezor/[email protected] filesystem Transitive: network +5 2.23 MB trezor-ci
npm/@trezor/[email protected] environment 0 17.7 kB trezor-ci
npm/@trezor/[email protected] None 0 1.09 MB trezor-ci
npm/@trezor/[email protected] None 0 11.8 kB trezor-ci
npm/@trezor/[email protected] None 0 38.7 kB trezor-ci
npm/@trezor/[email protected] network 0 165 kB trezor-ci
npm/@trezor/[email protected] None 0 2.89 kB trezor-ci
npm/@trezor/[email protected] None 0 64.2 kB trezor-ci
npm/@trezor/[email protected] None 0 234 kB trezor-ci
npm/@types/[email protected] None 0 8.92 kB types
npm/@types/[email protected] None 0 1.36 MB types
npm/@types/[email protected] None 0 18.9 kB types
npm/[email protected] network 0 43.7 kB fengmk2
npm/[email protected] None +1 1.57 MB ealmansi
npm/[email protected] None 0 55.7 kB no2chem
npm/[email protected] None 0 7.79 kB dcousens
npm/[email protected] None 0 4.38 kB dcousens
npm/[email protected] None +1 830 kB fanatid
npm/[email protected] None 0 34.6 kB volovyk-s
npm/[email protected] None +1 274 kB ealmansi
npm/[email protected] None 0 11.2 kB sindresorhus
npm/[email protected] None 0 315 kB stefanpenner
npm/[email protected] None 0 7.76 kB digitaldesignlabs
npm/[email protected] None 0 14 kB indexzero
npm/[email protected] None 0 125 kB nickyout
npm/[email protected] None 0 3.66 kB dead_horse
npm/[email protected] None 0 22.2 kB kawanet
npm/[email protected] network +1 922 kB tedeh
npm/[email protected] None 0 75.6 kB tdegrunt
npm/[email protected] None 0 177 kB dcode
npm/[email protected] None 0 422 kB kkoopa
npm/[email protected] None 0 4.28 kB dcousens
npm/[email protected] None +1 56.7 kB jst5000
npm/[email protected] None +1 1.18 MB jst5000
npm/[email protected] None 0 24.4 kB jst5000
npm/[email protected] None 0 147 kB intelliot
npm/[email protected] network +3 5.09 MB intelliot
npm/[email protected] None 0 182 kB artmllr
npm/[email protected] None 0 79.4 kB arv
npm/[email protected] None 0 1.1 MB junderw
npm/[email protected] None 0 19.1 kB dcousens
npm/[email protected] None 0 112 kB faisalman
npm/[email protected] None +1 7.3 MB thegecko
npm/[email protected] None 0 5.49 kB junderw
npm/[email protected] None 0 4.67 kB junderw

🚮 Removed packages: npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected], npm/@babel/[email protected]

View full report↗︎

Copy link

socket-security bot commented Sep 12, 2024

👍 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.

Ignoring: npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/@solana/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/@solana/[email protected], npm/@trezor/[email protected], npm/@trezor/[email protected], npm/@trezor/[email protected]

View full report↗︎

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 ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

@metamaskbot
Copy link
Collaborator

Builds ready [e3aa795]
Page Load Metrics (1516 ± 98 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint22522941391419201
domContentLoaded13562191149718991
load13642283151620498
domInteractive195731136
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 5.55 KiB (0.07%)

@vthomas13 vthomas13 requested a review from danjm September 12, 2024 21:52
@metamaskbot
Copy link
Collaborator

Builds ready [34f33b3]
Page Load Metrics (1805 ± 132 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint22527371727446214
domContentLoaded139626811776278133
load144826901805275132
domInteractive127528147
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 5.55 KiB (0.07%)

Copy link

codecov bot commented Sep 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.95%. Comparing base (9459903) to head (9ce641e).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #27112   +/-   ##
========================================
  Coverage    69.95%   69.95%           
========================================
  Files         1441     1441           
  Lines        50099    50099           
  Branches     14007    14007           
========================================
  Hits         35046    35046           
  Misses       15053    15053           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vthomas13 vthomas13 force-pushed the marty-trezor-update branch 3 times, most recently from 8804fcc to 60175c2 Compare September 13, 2024 16:26
@danjm
Copy link
Contributor

danjm commented Sep 13, 2024

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

Policies updated

@vthomas13
Copy link
Contributor Author

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

No policy changes

@metamaskbot
Copy link
Collaborator

Builds ready [a840f33]
Page Load Metrics (1727 ± 72 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint22222241521553265
domContentLoaded15102215171515373
load15232224172715172
domInteractive1310235199
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 5.55 KiB (0.07%)

Copy link

@metamaskbot
Copy link
Collaborator

Builds ready [9ce641e]
Page Load Metrics (1751 ± 154 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint20924261344659316
domContentLoaded143424591736317152
load144324661751320154
domInteractive1998382010
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 5.55 KiB (0.07%)

@danjm
Copy link
Contributor

danjm commented Sep 19, 2024

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

No policy changes

@metamaskbot
Copy link
Collaborator

Builds ready [8094f79]
Page Load Metrics (1844 ± 180 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint151932551841369177
domContentLoaded151231151812346166
load152032771844375180
domInteractive13187473718
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 5.55 KiB (0.07%)

@vthomas13
Copy link
Contributor Author

@SocketSecurity ignore npm/[email protected] npm/[email protected] npm/[email protected] npm/@solana/[email protected] npm/@trezor/[email protected]

npm/[email protected]: Verified the new author as a member of other open source projects
npm/[email protected], npm/[email protected], npm/@solana/[email protected], npm/@trezor/[email protected]: These are not used in trezor/connect-web, which is what we import.

@vthomas13
Copy link
Contributor Author

@SocketSecurity ignore npm/@trezor/[email protected] npm/@trezor/[email protected]
npm/@trezor/[email protected], npm/@trezor/[email protected]: These are not used in trezor/connect-web, which is what we import.

@vthomas13 vthomas13 force-pushed the marty-trezor-update branch 2 times, most recently from 145a9aa to ba14308 Compare October 7, 2024 14:48
@metamaskbot
Copy link
Collaborator

Builds ready [ba14308]
Page Load Metrics (1991 ± 112 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint30926161913434208
domContentLoaded165126051963235113
load169926301991233112
domInteractive28194644923
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 5.55 KiB (0.07%)

@vthomas13 vthomas13 requested a review from a team October 9, 2024 14:13
@vthomas13 vthomas13 force-pushed the marty-trezor-update branch from ba14308 to fd765ae Compare October 9, 2024 14:13
@metamaskbot
Copy link
Collaborator

Builds ready [fd765ae]
Page Load Metrics (1871 ± 95 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint16362300187020096
domContentLoaded16212291184318991
load16292299187119895
domInteractive247644157
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 5.55 KiB (0.07%)

@vthomas13 vthomas13 added this pull request to the merge queue Oct 10, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 10, 2024
@vthomas13 vthomas13 added this pull request to the merge queue Oct 10, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 10, 2024
@vthomas13 vthomas13 force-pushed the marty-trezor-update branch from fd765ae to 2b63afb Compare October 11, 2024 12:52
@vthomas13
Copy link
Contributor Author

@SocketSecurity ignore npm/[email protected] npm/[email protected] npm/[email protected]
The new authors are members of other related open source projects

@vthomas13 vthomas13 force-pushed the marty-trezor-update branch from 2b63afb to ebc4ff9 Compare October 11, 2024 14:31
Copy link

@metamaskbot
Copy link
Collaborator

Builds ready [ebc4ff9]
Page Load Metrics (1869 ± 121 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint151023931862253122
domContentLoaded150323841831252121
load151123951869252121
domInteractive24253615627
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 5.55 KiB (0.07%)

@vthomas13 vthomas13 added this pull request to the merge queue Oct 11, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 11, 2024
@vthomas13 vthomas13 added this pull request to the merge queue Oct 11, 2024
Merged via the queue into develop with commit 39e0251 Oct 11, 2024
79 checks passed
@vthomas13 vthomas13 deleted the marty-trezor-update branch October 11, 2024 16:12
@github-actions github-actions bot locked and limited conversation to collaborators Oct 11, 2024
@metamaskbot metamaskbot added the release-12.7.0 Issue or pull request that will be included in release 12.7.0 label Oct 11, 2024
@gauthierpetetin gauthierpetetin added release-12.6.0 Issue or pull request that will be included in release 12.6.0 and removed release-12.7.0 Issue or pull request that will be included in release 12.7.0 labels Oct 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.6.0 Issue or pull request that will be included in release 12.6.0 team-extension-platform
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants