Can't connect to Trader Joe if wallet is set as a default #3412
Labels
Type: Bug
Something isn't working
Comments
kkosiorowska
pushed a commit
that referenced
this issue
Jul 13, 2023
…cases (#3546) ## Background Up until now, Taho's approach to being marked as default was meant to be roughly: - If Taho is marked as default, and the dApp supports showing a non-MetaMask provider at `window.ethereum`, make Taho that provider. - If Taho is marked as default, and the dApp *doesn't* support showing a non MetaMask provider, have Taho impersonate MetaMask so that users can still connect using Taho. - If Taho is marked as not-default, leave everything untouched, and only show Taho for dApps that specifically look for it (typically at `window.tally`, but sometimes in `window.ethereum.providers`). For this to work, the window provider was tracking an allowlist of sites to impersonate MetaMask on, and it was *also* tracking an allowlist of sites to do additional tricks on (e.g. returning a blank `providers` array for sites that detect MetaMask and don't look for any other wallet). There were other tricks also being done to try to discern between scenarios, etc. Other wallets just throw their hands up and attempt to impersonate MetaMask most/all the time, leading to escalating wars with dApp frameworks to detect these behaviors. ## Revised Approach This PR throws all of that away. It was confusing for users (do you click on MetaMask? Browser/Injected? What happens if Taho isn't default? What about the times when MetaMask impersonation isn't hardcoded for a site?), and it was difficult to maintain (allowlists of specific sites are not realistically a scalable long-term approach). Instead, having Taho set as default (a reminder that as of #3492, this is represented in the UI as a toggle between “connect with MetaMask” and “connect with Taho”) should mean two things: - Trying to connect to MetaMask will *always* use Taho. - For dApps that also specifically detect Taho, they may *also* allow connecting with Taho. If Taho is set as default, attempting to connect with MetaMask will still connect with Taho. On the flip side, if Taho is not set as default, it should mean two things: - Trying to connect to MetaMask will *always* use whatever was presenting as MetaMask before Taho got involved. - For dApps that also specifically detect Taho, they will allow connecting with Taho still. ### Technical Details How we do this is straightforward: any provider on the `providers` array that presents as MetaMask (our heuristic: `isMetaMask` returns `true` AND there are no other `is*` methods on the provider) is wrapped in a one-time proxy. That proxy passes everything through to the wrapped provider, *unless* Taho has been set to present as MetaMask by the user (an explicit choice). In this case, a subset of functions are always redirected to Taho, including account and RPC requests. Additionally, any calls to `isMetaMask` on `window.ethereum` return `true` if Taho is set to default, even though the provider presented on `window.ethereum` is the Taho provider. We do two more small things: - When Taho is set as default, it is set as the first provider in the providers array. When it is not, it is removed from the front of the providers list. This allows MetaMask or other wallet usage for dApps that use a framework like wagmi, which especially in older versions only used the first wallet in the providers array for connections. Examples of dApps that show this behavior include Lens and Vela (e.g. see [this bug report in Discord](https://discord.com/channels/808358975287722045/888500174685614090/1127318749746311258)). - When Taho is set as default by the user and MetaMask is not installed, a mock MetaMask is presented to the dApp that reroutes calls to Taho. If Taho is switched back to not default, the mock is removed. This allows dApps like bitcoinbridge.network that only allow connecting via MetaMask to work with Taho even if MetaMask is not installed. The net result is that the “act as MetaMask”/“Taho as default” toggle should behave consistently across all dApps. Famous last words :) Finally, we fix a small issue with eth_requestAccounts handling on certain dApps. Not related to MetaMask impersonation really, but knockin' bugs out. ## Testing - This is going to require doing some relatively extensive testing in our release QA dApps to make sure that MetaMask is replaced when Taho is set as default and not when it is not. - All these should also be replicated with Taho as default and not and MetaMask disabled (to ensure the dApp registers MetaMask as uninstalled when Taho is not default, and allows interaction as MetaMask when Taho is default). - We should also check Lens (https://claim.lens.xyz/) to make sure the Browser connection uses Taho when default, and MetaMask when not. - Finally, the dApps in the issues below should be tested to make sure they're all working! Fixes #3433, fixes #3412, fixes #3368, fixes #3242, fixes #3231, fixes #2506, fixes #2706, fixes #2818. Latest build: [extension-builds-3546](https://github.com/tahowallet/extension/suites/14241880950/artifacts/799754754) (as of Wed, 12 Jul 2023 12:06:52 GMT).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
and others
Problem with connection on Trader Joe:
If Taho is set as not default then it is listed as a Browser wallet option:
If Taho is set as default wallet then it should be available under Metamask option but it is no longer working. Browser wallet option is gone too.
The text was updated successfully, but these errors were encountered: