search

keep-network / tbtc-dapp

Deposit BTC and redeem TBTC
http://dapp.test.tbtc.network/
MIT License
33 stars 31 forks source link

tBTC dApp reads MEW as MetaMask #251

Open JaviAnibarro opened 4 years ago

JaviAnibarro commented 4 years ago

Description:

When I was trying to connect my wallet on tBTC dApp, it asked me to connect my MetaMask wallet but after I clicked it MEW wallet popup came out. And after clicking the address I have on MEW wallet, it appeared there was nothing happened, it's stuck on that popup.

Steps to reproduce the behavior:

  1. Go to 'https://dapp.test.tbtc.network/deposit'
  2. Changes MetaMask settings(this can read and change site data) from on all sites to when you click the extension and unpin it
  3. Click Connect to a wallet on 'https://dapp.test.tbtc.network/deposit'
  4. See error

Expected behavior:

tBTC dApp should not show a wrong app when I am trying to connect my wallet

Environment details:

OS: Windows 10 , 64 bit OS
Web Browser: Google Chrome (64-bit)
Wallet: Metamask (Chrome extension), and MEW wallet

Step-Meta Step-MEW Step-Stuck

mhluongo commented 4 years ago

Hey @JaviAnibarro, we only support MetaMask right now, though we have Ledger and a few others in the works. I'm moving this from "bug" to "enhancement".

If you have examples of dApps that handle MEW or any other favorite wallets of yours well, screens and links would be handy!

JaviAnibarro commented 4 years ago

@mhluongo Ok But why does it load MEW instead of asking for permission to load MetaMask? or why do not the dApp shows a notification that we do not have MetaMask installed when MEW is not installed? I tried to do this too before using Brave browser(without MEW installed) and it directly told me that I do not have MetaMask installed. But here instead of showing this notification, the dApp shows MEW.

mhluongo commented 4 years ago

That really depends on how your web3 is injected, as far as I know? See here.

I think supporting web3 generically before explicit multi-wallet support makes sense- otherwise we're excluding non-MetaMask users unnecessarily. What do you think?

JaviAnibarro commented 4 years ago

are not one of the points of this dApp is mainly about the usage of MetaMask itself? what will happen for those (who installed both MEW and MetaMask) when they encounter this problem and do not know how to fix it? do not you think we are creating an unnecessary problem here? and try to consider the fact the newbies that trying to understand it or simply trying to connect their wallet and stuck at this deposit progress, will not they think the dApp has a problem and unusable?

what is the advantage of allowing non-MetaMask to use it if they can not connect the wallet? If we make a clear line to non-MetaMask wallet can not click it or shows the notification they do not have MetaMask installed like what I saw on Brave browser. Will not it help them and make things easier? also with it, we can encourage them to use supported extension.

another thing that is worrying is, what will happen if it reads another extension(it is possible?)?

mhluongo commented 4 years ago

I'm struggling to understand that alternative behavior you'd like to see, before we roll out multi-wallet support.

JaviAnibarro commented 4 years ago

@mhluongo well nvm, about dApp supports MEW. you can check this one for example: https://ethermon.io/

Shadowfiend commented 4 years ago

The issue here as I read it is that you have BOTH Metamask AND MyEtherWallet installed, but when you try to connect to Metamask you get MEW instead, and then it doesn't work. Is that a correct understanding of what you ran into?

JaviAnibarro commented 4 years ago

Yes @Shadowfiend , it stuck after MEW came out.