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

[Bug]: Connect to MetaMask/Brave not accurate #1250

Closed
1 task done
mario-iliev opened this issue Sep 5, 2022 · 4 comments
Closed
1 task done

[Bug]: Connect to MetaMask/Brave not accurate #1250

mario-iliev opened this issue Sep 5, 2022 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@mario-iliev
Copy link

Current Behavior

Hey guys. I stumbled upon a problem which I tried to detect in the codebase but I'm not really sure where is the correct place to do it, so I can't provide PR for it.
When there is MetaMask and Coinbase wallet installed on Brave browser and I choose to connect to MetaMask trough
onboard .connectWallet({ autoSelect: { label: 'MetaMask', disableModals: true }, })
I'm being connected to Brave Wallet instead. I suspect that there is a loop check in the window.ethereum.providers and the first which has isMetaMask === true is selected but there is no specific check for "isBraveWallet". From the official onboard connect modal I see that there is no differentiation between MetaMask and Brave wallet but I really think there should be.

Expected Behavior

Target correctly MetaMask and Brave wallet connection.

Steps To Reproduce

  1. Install the following injected wallets in a Brave browser: Metamask and Coinbase
  2. Try to connect to Metamask

Result: You will be connected to Brave Wallet but the modal will state that it's MetaMask

What package is effected by this issue?

@web3-onboard/core

Is this a build or a runtime issue?

Runtime

Package Version

2.8.1

Node Version

16.14.2

What browsers are you seeing the problem on?

No response

Relevant log output

No response

Anything else?

No response

Sanity Check

  • If this is a build issue, I have included my build config. If this is a runtime issue, I have included reproduction steps and/or a Minimal, Reproducible Example.
@Adamj1232
Copy link
Member

@mario-iliev thanks for reporting! Looking into this now. I will update here as development progresses.

@Adamj1232
Copy link
Member

@mario-iliev I have a PR with the fix created. If interested you can check it out here - #1258

@Adamj1232
Copy link
Member

This fix is available with injected package 2.2.2-alpha.2 and will be released early next week

@Adamj1232
Copy link
Member

Closing as this fix is available with the injected-wallet package v2.2.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants