extension icon indicating copy to clipboard operation
extension copied to clipboard

▦ Orthogonal Networks Season 2: Don't change network for the dapps when the network for the extension changes

Open greg-nagy opened this issue 2 years ago • 9 comments

save network for dapps on first encounter

Switching networks in the extension shouldn't impact different dApp connections.

Without this the network is changed for dapps with the network selection in the extension until dapp's networkswitcher is used for the first time.

Test

  • [x] Visit app.uniswap.org and connect to Tally Ho
  • [x] Change Tally Ho to Polygon, and make sure Uniswap hasn't switched networks
  • [x] Change Tally Ho back to Ethereum, then change Uniswap to Polygon. Ensure the extension is still set to mainnet
  • [x] Change network in uniswap. Ensure the network in the extension is unaffected.
  • [x] Change accounts in the extension. Ensure that the account is changed for uniswap.

Closes #1846

This PR builds on the findings of #1961 and #1963

greg-nagy avatar Aug 17 '22 13:08 greg-nagy

cc @mr-michael

These thoughts and concerns are also valid here: https://github.com/tallycash/extension/pull/1961#issuecomment-1216982723

TL;DR: This change will remove the extension network <> dapp network connection. This is a good thing for those dapps with their own network selector. For those that don't have that, it will remove the possibility of changing the network.

Is this something that we are ok with?

greg-nagy avatar Aug 17 '22 13:08 greg-nagy

Yay!!!

mhluongo avatar Aug 17 '22 16:08 mhluongo

orthogonal-networks-test

mhluongo avatar Aug 17 '22 17:08 mhluongo

(Despite my comment trying to lay out the options linked above, I'm 100% in favor of this approach.)

Shadowfiend avatar Aug 18 '22 01:08 Shadowfiend

I have this behavior, if you connect and disconnect and then try to connect again but you start on a different network - uniswap is changing it's network, it is happening even after wallet reinstall so maybe it is on Uniswap side?

https://user-images.githubusercontent.com/20949277/185379511-04291d91-1bf1-4367-9ac3-e7314281cf1f.mov

jagodarybacka avatar Aug 18 '22 11:08 jagodarybacka

I think this is happening because we persist the dapp connections in indexdb and don't flush them if an account is removed - probably something we should do tbh (but not as part of this PR IMO).

0xDaedalus avatar Aug 18 '22 13:08 0xDaedalus

FYI I think merging this would prevent users from changing networks on sushiswap.

0xDaedalus avatar Aug 18 '22 20:08 0xDaedalus

I have this behavior, if you connect and disconnect and then try to connect again but you start on a different network - uniswap is changing it's network, it is happening even after wallet reinstall so maybe it is on Uniswap side?

It's because we store the currently active network in extension as a default for the dapp almost immediately on dapp load. After this, the only way to change the network for the dapp is through its own network switcher. But the network switcher won't have any effect until permission is granted for the dapp.

  • Fresh install > Import wallet
    • set wallet to polygon > load app.uniswap > grant permission > uniswap will be on polygon
    • wallet is on ethereum > load app.uniswap > change network on uniswap to polygon > change network in wallet to polygon > connect wallet/grant permission > uniswap is on ethereum

To fix this we either need to allow wallet_switchEthereumChain without permission or delay the settings of the network for a dapp to a later point in the flow. Maybe after permission is granted. Until then the dapp uses the active network in the wallet.

greg-nagy avatar Aug 19 '22 06:08 greg-nagy

would prevent users from changing networks on sushiswap

@0xDaedalus but even on current main sushiswap is only working on Ethereum

jagodarybacka avatar Aug 24 '22 11:08 jagodarybacka

this PR should have been closed long ago ... because it stores info from every visited website in indexeddb

also #2314 implemented the proper fix

greg-nagy avatar Nov 07 '22 14:11 greg-nagy