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] Upgrade Trezor Connect to the latest #8973

Closed
angelcheung22 opened this issue Mar 18, 2024 · 0 comments
Closed

[Trezor] Upgrade Trezor Connect to the latest #8973

angelcheung22 opened this issue Mar 18, 2024 · 0 comments

Comments

@angelcheung22
Copy link

Related Git thread from client. MetaMask/metamask-extension#10896
Related git issue from previous thread:MetaMask/eth-trezor-keyring#187

Issue

MetaMask Pop-up Window Always Unexpectedly Closes Right Before Entering PIN on Trezor Wallet

Info

The following change in Trezor Connect 9.1.1 introduces a mitigation about the auto-closing of popups.

feat(connect-popup): when a call to TrezorConnect returns success: false popup remains opened and displays error page instead.

This is driven by the client, and requires an NPM upgrade. 

This Issue exist since 2021, and It still happens when using Trezor and chrome.

Trezor Connect was updated and the issue is fixed. Metamask, however, has to manually update npm package otherwise it won't work

Remove redundant patches and support any documentation updates as required

Patches like this can be removed. Since the missing dependency is available on npm.

Technical Breakdown

Latest Trazor/connect@9.1.6 contain following sub libraries which has been used by @metamask/eth-trezor-keyring library

@trezor/connect-web@9.0.6 ---> @9.1.6
@trezor/connect-plugin-ethereum@9.0.1. ---> @9.0.2
So far i manually upgrade the library and run yarn test unit tests, and all tests still pass which is good side that existing feature didnt break after upgrade the library. however, we still need to build the latest @metamask/eth-trezor-keyring library with latest @trazor/connect@9.1.6 and use it to metamask-extension to test the issue fix or not.

Following potential tasks will be involved in this tasks.

upgrade above two library in eth-trezor-keyring repo. (estimate: 1 hour)
(optional) Modify trezor-bridge.ts, trezor-connect-bridge.ts and trezor-keyring.ts code if API issue found from above upgrade (so far unit tests pass, which mean we may not need to do this, however, we need to double check). (estimate: 1 day)
extend trezor-connect-bridger.test.ts unit tests if any senario missing. (esitmate: 0.5 day)
build @metamask/eth-trezor-keyring and apply it to metamask-extension and tests ( estimate: 2 hours)
(optional) if issue still exist after testing, we may need to have some code fix in metamask-controller.js and hardware-keyring-builder-factory.js (estimate: 2 days)
So total estimate for this tasks will be 3.5 days to 4 days,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants