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

feat: improve mm impersonator checks #5896

Merged
merged 5 commits into from
Jan 30, 2024

Conversation

gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Dec 19, 2023

Description

Adds a "definitive" (for now at least) list of MM impersonators and explicitly checks against them with a new checkIsMetaMaskImpersonator method in addition to our checkIsMetaMask method, to avoid impersonators being detected as MM.

Pull Request Type

  • πŸ› Bug fix (Non-breaking Change: Fixes an issue)
  • πŸ› οΈ Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
  • πŸ’… New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

Risk

  • MM could be wrongly detected as an impersonator and the snap modal may not be displayed for it, test according to the testing steps

Testing

  • When connecting Rabby wallet (or any of the wallets in the METAMASK_IMPERSONATORS e.g Zerion wallet extension), the MM snaps modal doesn't show up
  • When connecting actual MM, the MM snaps modal shows up
  • Connecting previously not working MM impersonators (e.g Zerion) is now possible. Note, this is not a magic solution that will make any impersonator work as they all have their ramifications. Tested Zerion is now happy after feat: gracefully handle mm impersonatorsΒ hdwallet#657, but if other impersonators are still not working, this will need to be tackled as a per-wallet basis.

Engineering

  • ☝🏽

Operations

  • ☝🏽

Screenshots (if applicable)

MM snaps modal only shows up for actual MM

  • Develop 🚫 - modal shows up for MM, but also impersonators
image image Screenshot 2023-12-19 at 13 10 10 Screenshot 2023-12-19 at 13 10 17
  • This diff βœ… - modal shows up only for actual MM
image image image

Copy link
Contributor Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@gomesalexandre gomesalexandre changed the title feat: mm impersonator checks feat: improve mm impersonator checks Dec 19, 2023
.yarnrc.yml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@0xApotheosis 0xApotheosis marked this pull request as ready for review January 30, 2024 04:34
@0xApotheosis 0xApotheosis requested a review from a team as a code owner January 30, 2024 04:34
Copy link
Contributor

@0xApotheosis 0xApotheosis left a comment

Choose a reason for hiding this comment

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

I'm getting a snaps install prompt with using actual metamask, but not when using Brave Wallet πŸ‘Œ

@0xApotheosis 0xApotheosis enabled auto-merge (squash) January 30, 2024 05:16
@0xApotheosis 0xApotheosis merged commit 97e2c34 into develop Jan 30, 2024
4 checks passed
@0xApotheosis 0xApotheosis deleted the feat_mm_impersonators_checks branch January 30, 2024 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants