Repeater Connection: Forward requests to new default when default is switched off during dApp connection flow #3462
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Shadowfiend
force-pushed
the
repeater-connection
branch
from
c9f4f39
to
7e1f0e8
Compare
6 tasks
This largely doesn't matter, but we were pushing a notification to all pages when a single permission was revoked, even if that permission had not been granted in the first place. This is a tiny step in improving the permission revocation process.
When called, this pushes an RPC request to the current provider, which has ostensibly been previously updated. It is meant to be used to re-forward to MetaMask or another wallet when Taho disables its default status during an interaction.
In addition to re-routing all in-flight requests to the new wallet when the Taho-as-default toggle is flipped, we also no longer reload the page for any dApps.
The way that the provider proxy was set up, changing the current provider would not route new requests to that provider, instead requiring the dApp to reconnect. To allow transparent forwarding to the new default when the connection switches, the proxy now always forwards to the current provider (when available).
When currentProvider is unset on the router, return `undefined` for `window.ethereum` as well.
Only when updated dApp connections are enabled, the dApp connection popup auto-closes (after a 300ms delay to let the user see the toggle animate). The expectation is that the new default wallet will pop up immediately as the window provider replays the connection request from Taho to the new default.
Now that the MetaMask fallback is more seamless, make it so that Taho behaves as MetaMask on all sites when it is set as default. Only when updated dApp connections are enabled.
Shadowfiend
force-pushed
the
repeater-connection
branch
from
7e1f0e8
to
0da8205
Compare
hyphenized
suggested changes
Jun 15, 2023
| ) { | ||
| // When default wallet is toggled off, wait for the toggle animation to | ||
| // complete and close the window out. | ||
| setTimeout(() => window.close(), 300) |
There was a problem hiding this comment.
I think this is causing an issue when Taho is the only wallet available. On https://pancakeswap.finance/ when Taho is not the default, connecting with the injected option causes the popup to close immediately.
There was a problem hiding this comment.
Incidentally this shouldn't trigger in that scenario—it should really only trigger when the user actively toggles it off, i.e. on transition. Let me noodle on this a little.
If this isn't causing that behavior with the feature flag off, let's not let it block the merge since there's a follow-up in the mix.
The resolvers were being cleared immediately, then being cleaned up again one by one during the rerouting. Let the initial clearing do the work.
Merged
jagodarybacka
added a commit
that referenced
this pull request
Jun 23, 2023
## What's Changed * Repeater Connection: Forward requests to new default when default is switched off during dApp connection flow by @Shadowfiend in #3462 * Auto-Not-So-Matic: Fix two matic.network URLs to polygon.technology by @Shadowfiend in #3483 * v0.38.0 by @kkosiorowska in #3480 * Case Dismissed: Forcibly show DAppConnectionInfoBar popover on first dApp connection by @Shadowfiend in #3464 * Add Hardhat Fork Functionality by @0xDaedalus in #3247 * Faded Jeans: Rename fadeIn class to fade_in by @Shadowfiend in #3485 * Fix issue for discovery transaction hash by @kkosiorowska in #3458 * Full Sweep: Drop the USE_UPDATED_SIGNING_UI feature flag by @Shadowfiend in #3475 * Token Discovery - Remap redux asset balances by @hyphenized in #3195 * Make specific warnings for adding custom asset by @kkosiorowska in #3478 * Run NFTs e2e tests on a controlled wallet address by @michalinacienciala in #3487 * v0.38.1 by @jagodarybacka in #3484 **Full Changelog**: v0.38.1...v0.39.0 Latest build: [extension-builds-3496](https://github.com/tahowallet/extension/suites/13792957673/artifacts/764738599) (as of Thu, 22 Jun 2023 14:27:32 GMT).
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds the ability to pass through to the next provider as soon
as the user toggles default wallet off.
Can be reviewed and merged separately from #3451.
Ex:
its-alive.mov
Testing
Note that you can get into a state where you've previously requested a MetaMask connection, but did not complete the flow (e.g. by not unlocking MetaMask). In this case MetaMask will auto-decline the connection request from the dApp, so the above flow will appear not to work. Make sure MetaMask is in an unlocked idle state before testing the flow above.
Testing Env
Latest build: extension-builds-3462 (as of Thu, 15 Jun 2023 15:12:02 GMT).