swapr-dapp
swapr-dapp copied to clipboard
SwaprHQ
feature/upgrade-wallet-library
Summary
Upgrade web3-react library from v6 to v8. This should fix most of bugs/issues caused by multiple wallets used in the same time. Also prepare dapp for new wallets integration (like TallyHo!) and support for injected wallets (like Brave Wallet).
Upgrading can not cause regression, so swapr functionality and usage should be as expected and out of bugs.
Fixes #1440 Fixes #1133 Fixes #940 Fixes #699
Related to: #649 and #1169. Will be completed after this is merged. Related but needs to be investigated: #1439
To Test
- [ ] connect with each wallet
- [ ] change wallet
- [ ] disconnect wallet
- [ ] green indicator should show only one wallet which is currently in use (even if the user was connected with a few different ones)
- [ ] if user left the swapr after coming back wallet should be connected automatically and be the same as last one
- [ ] change account within one wallet (can be done by wallet extension not from swapr level), eg create another account in MetaMask - account shown in swapr should be the same
- [ ] switch network if connected to unsupported network
- [ ] switch/disconnect wallet if connected to unsupported network
- [ ] add supported network not listed in wallet extension
- [ ] modals/popups and information shown on them are proper
Background
For a long time swapr has been struggling with some wallet-dependant issues, like conflicts between injected connectors used the same time (Brave Wallet) or problems with network switching. After a few temporary fixes we decided to use the newest version of wallet library named web3-react. Although this one is still tagged as beta it is reccomended by authors to upgrade, mainly because of highly modular design, known bugs fixes and long term support (also for new third-party connectors). This means for us easier integration for wallets like TallyHo! or BitKeep. More information can be found on web3-react github: https://github.com/Uniswap/web3-react
Deploy Preview for swapr ready!
| Name | Link |
|---|---|
| Latest commit | 5a66c661ba1fb9aee610b4c09808ba6cb1aa6b0a |
| Latest deploy log | https://app.netlify.com/sites/swapr/deploys/63935ed4877f0a0009138d73 |
| Deploy Preview | https://deploy-preview-1330--swapr.netlify.app/ |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
Tested, the condition is much better than it was, it is possible to connect different wallets, it works normally when swapping, it always opens the appropriate wallet and there is no more problem with changing. But there are still some bugs that need to be fixed:
After several wallet changes, it happens that the indicators show that the user is connected to both:
Also, after several wallet changes, it is not possible to disconnect all wallets, it is only possible to change Coinbase or Metamask.
It also happens that sometimes the 'Change Wallet' button doesn't work and the 'Disconnect wallet' doesn't react after clicking:
Thank you @MilanVojnovic95 for tests and feedback!
Tested, the condition is much better than it was, it is possible to connect different wallets, it works normally when swapping, it always opens the appropriate wallet and there is no more problem with changing. But there are still some bugs that need to be fixed:
After several wallet changes, it happens that the indicators show that the user is connected to both:
This is expected behavior, consulted with Zett. Indicators show which wallet is connected with the dapp and indeed it is possible now to have more than one. This is the way how eg Uniswap works now. But also I agree that maybe would be good to show which wallet is really in use. TBD.
Also, after several wallet changes, it is not possible to disconnect all wallets, it is only possible to change Coinbase or Metamask.
It also happens that sometimes the 'Change Wallet' button doesn't work and the 'Disconnect wallet' doesn't react after clicking:
Swapr_05.08.webm
Nice catch! I'll try to reproduce and fix it!
When you connect to an un supported network. Like optimism.
May be we can hide the token and expeditions in that case
Great job 👏
I agree, it would be good to show which wallet is really in use. We can show the green bullets on the connected but the wallet in current use wallet could also have a different background or something, maybe @zamli has a suggestion.
I agree, it would be good to show which wallet is really in use. We can show the green bullets on the connected but the wallet in current use wallet could also have a different background or something, maybe @zamli has a suggestion.
I've restored indicator status as it was before (59ae72d), so at the moment one green dots points to the wallet which is really in use. I'm open for any suggestion and improvements though ;)
The ens domain and avatar is not displayed on rinkeby instead of wallet address
The ens domain and avatar is not displayed on rinkeby instead of wallet address
Thanks! I'll take a look and fix asap!
Looks like this pr fixed problems from #940 but #1439 is still present and #1440 is also occuring but in a litlle bit diffrent manner - the change network button appear but it's not clickable
There are few other problems:
-
There is some warning in the console from eslint
-
Testnets are not clickable when using coinbase wallet (chrome plugin)
Also i got a bunch of problems when i try to disconnect wallet through swapr (i got both mm and cb wallet installed)
https://user-images.githubusercontent.com/30408272/189310938-2c186dec-02ad-424d-ae92-b9e18c3c4664.mp4
When using mobile version of Coinbase network in Swapr should be switched after user approve it in app
I wasn't able to reproduce this error. Please let me know if you know the steps to reproduce.
When using mobile version of Coinbase network in Swapr should be switched after user approve it in app
In my case switching network both via Swapr and Coinbase Wallet App on phone works without need of approve. So I'm not sure if there's a problem from our side or Coinbase App.
Anyway I've made some test and all works fine. Networks can be switched and transactions can be approved as well, see scr below.
When using mobile version of Coinbase network in Swapr should be switched after user approve it in app
In my case switching network both via Swapr and Coinbase Wallet App on phone works without need of approve. So I'm not sure if there's a problem from our side or Coinbase App.
Anyway I've made some test and all works fine. Networks can be switched and transactions can be approved as well, see scr below.
Still got that problem, it may differ for me because i use ios version of Coinbase wallet
Still got that problem, it may differ for me because i use ios version of Coinbase wallet
This might be the case. I'll investigate it then with ios.
Still got that problem, it may differ for me because i use ios version of Coinbase wallet
Still the same, I wasn't able to reproduce this also on ios. Indeed, during network switching app doesn't wait for user's approval, but this is the Coinbase Wallet case (checked for different apps like Uniswap). Except this I couldn't find any other issues.
For now, i also don't see any major problems with this pr. I would accept that as mvp and merge it to develop