Skip to content
This repository has been archived by the owner on Oct 7, 2024. It is now read-only.

Add public key check to prevent freezing. #155

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

yuki-js
Copy link

@yuki-js yuki-js commented Jan 21, 2023

Explanation

Fixes MetaMask/metamask-extension#12967

When the selected account on MetaMask is changed, the public key provided by MetaMask will differ from the one that the TrezorKeyring remembers. As a result, it will search for the address in the incorrect address space and eventually fall into a nearly infinite loop.

To fix this issue, I implemented a public key check. To do this, I made the following changes:

  • Made the _pathFromAddress function asynchronous
  • Added an assertion to ensure that the public key is not changed
  • Fixed test

Pre-merge author checklist

  • I've clearly explained:
    • What problem this PR is solving
    • How this problem was solved
    • How reviewers can test my changes
  • Sufficient automated test coverage has been added

Pre-merge reviewer checklist

  • Manual testing (e.g. pull and build branch, run in browser, test code being changed)
  • PR is linked to the appropriate GitHub issue
  • IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature. The key has expired.
colombod Diego Colombo

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@yuki-js yuki-js requested a review from a team as a code owner January 21, 2023 05:08
@legobeat legobeat requested review from danjm and darkwing August 30, 2023 06:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metamask freezes when trying to confirm transaction on Trezor T
3 participants