Skip to content
This repository was archived by the owner on Jun 24, 2022. It is now read-only.

Enable Coinbase Wallet (wallet-link) #2353

Merged
merged 4 commits into from
Feb 14, 2022
Merged

Enable Coinbase Wallet (wallet-link) #2353

merged 4 commits into from
Feb 14, 2022

Conversation

henrypalacios
Copy link
Contributor

Summary

Closes #432
Enable the ability to connect via Coinbase wallet through browser extension and mobile app
Selection_437

To Test

  • Go to the Connect to a wallet button and click on the Coinbase Wallet option.

Background

This PR is part of the activation of previously supported wallets #2165

@henrypalacios henrypalacios self-assigned this Jan 28, 2022
@henrypalacios henrypalacios added app:CowSwap CowSwap app Protofire Handled by Protofire development team labels Jan 28, 2022
@henrypalacios henrypalacios mentioned this pull request Jan 28, 2022
Merged
2 tasks
@github-actions
Copy link
Contributor

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@henrypalacios henrypalacios force-pushed the 414-enable-wallet-link branch from bd7ef93 to 9a1e407 Compare January 28, 2022 18:45
@henrypalacios henrypalacios changed the base branch from develop to 2165-adding-wallet-supports January 29, 2022 00:27
@elena-zh
Copy link

Hey @henrypalacios , great PR!
I was able to run transactions in Rinkeby and Gnosis chain using Coinbase wallet iOS app, iOS app integrated browser and Chrome web extension.
Some minor issues:

  1. Can't connect to the Coinbase wallet in iOS #1950 issue is still not fixed: I can't connect to coinbase wallet using wallet connect
    image
    But maybe we could leave it as it is for now and fix it in another PR

  2. I can't disconnect Coinbase wallet inside the CowSwap app. I mentioned about it here Can't Disconnect Metamask/Coinbase wallet #227 (comment)

  3. When disconnect Coinbase wallet inside the app, Coinbase connection window is displayed, and it is impossible to change connection until a user closes this window. However, it might be not a bug, just feature... @henrypalacios , @alfetopito , am I right?
    image

  4. The app is not disconnected when lock the wallet:
    image

  5. The app forces to connect to it when sign out of the account. In order to connect to another wallet, user needs to double refresh the Cowswap app
    image

  6. The app is still connected to the Coinbase wallet when disable its extention in the Chrome settings (until a user clears out all the app local storage)
    image

@alfetopito
Copy link
Contributor

Hey @henrypalacios , great PR! I was able to run transactions in Rinkeby and Gnosis chain using Coinbase wallet iOS app, iOS app integrated browser and Chrome web extension. Some minor issues:

1. [Can't connect to the Coinbase wallet in iOS #1950](https://github.com/gnosis/cowswap/issues/1950) issue is

That's is not an issue because coinbase wallet uses another "wallet connect like" standard called Wallet link, and it won't work with Wallet connect.

2. I can't disconnect Coinbase wallet inside the CowSwap app. I mentioned about it here [Can't Disconnect Metamask/Coinbase wallet #227 (comment)](https://github.com/gnosis/cowswap/issues/227#issuecomment-1025881116)

Same here, but out of the scope of this PR.

3. When disconnect Coinbase wallet inside the app, Coinbase connection window is displayed, and it is impossible to change connection until a user closes this window. However, it might be not a bug, just feature... @henrypalacios , @alfetopito , am I right?
   ![image](https://user-images.githubusercontent.com/70885163/151822431-43102dcd-db93-4efe-9d22-232f11088c1f.png)

This worked fine for me.
After disconnecting I see coinbase modal, closing it I see the wallet modal where I can click back to see all the wallets again
Screen Shot 2022-02-02 at 10 39 55
Screen Shot 2022-02-02 at 10 40 12

4. The app is not disconnected when lock the wallet:
   ![image](https://user-images.githubusercontent.com/70885163/151821092-efe3e85b-2fd2-4027-bafd-39b8f305cb0c.png)

I think that depends on the wallet to tell the UI. The same thing happens with Nifty. It behaves as connected even though without being unlocked.
I consider this a non-issue.

5. The app forces to connect to it when sign out of the account. In order to connect to another wallet, user needs to double refresh the Cowswap app
   ![image](https://user-images.githubusercontent.com/70885163/151821424-082936cb-a358-4c5e-acf4-8f0bd7244a23.png)

I did not experience that.
When signing out of the extension, it prompts me to sign on/create a new wallet, but that's up to the wallet.
On CowSwap it was disconnected and it didn't ask me to connect again.
I was in the swap page BTW, might be different in other places where the connection is required.

6. The app is still connected to the Coinbase wallet when disable its extention in the Chrome settings (until a user clears out all the app local storage)
   ![image](https://user-images.githubusercontent.com/70885163/151822055-a28e18dc-c8e0-4eb4-a7df-e56975233743.png)

I also think this is wallet's fault. If it doesn't tells us it's gone we can't tell until an action is triggered.

alfetopito
alfetopito approved these changes Feb 2, 2022
View reviewed changes
Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with the mobile app and browser extension, working great

@elena-zh elena-zh mentioned this pull request Feb 3, 2022
Closed
elena-zh
elena-zh approved these changes Feb 3, 2022
View reviewed changes
Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alfetopito , thanks for clarification.
@henrypalacios , approved!

@elena-zh elena-zh mentioned this pull request Feb 4, 2022
Open
@henrypalacios henrypalacios merged commit 2f27882 into 2165-adding-wallet-supports Feb 14, 2022
@henrypalacios henrypalacios deleted the 414-enable-wallet-link branch February 14, 2022 11:04
ramirotw pushed a commit that referenced this pull request Feb 18, 2022
@Sowiedu
style: Fix letter case consistency (#2353)
8ea5cb4 
* Fix letter case consistency

Fixed lower case to upper case to be consistent with:

src/pages/AddLiquidityV2/index.tsx
src/pages/RemoveLiquidity/index.tsx
src/pages/Swap/index.tsx
src/state/burn/hooks.ts
src/state/burn/v3/hooks.ts
src/state/mint/hooks.ts
src/state/mint/v3/hooks.ts
src/state/stake/hooks.ts
src/state/swap/hooks.ts

* style: Fix case to be consistent
alfetopito pushed a commit that referenced this pull request Mar 1, 2022
@henrypalacios
Use previously supported wallets (#2166)
e88eb5e 
* testing

* Enable Coinbase Wallet (wallet-link) (#2353)

* enable walletlink and upgrade package

* Limiting the supported networks for CoinbaseWallet

* Adding setDefaultInjected provider

* Enable Formatic wallet (#2388)

* Activate formatic wallet

* Adding reconnect Uninjected Provider

* Adding connecting web3Status after refresh

* Upgrade fortmatic to v2.2.1

* Mute unnecessary Overlay useEffect

* Adding new API 🔑

* Adding custom src/custom/connectors/Fortmaitc.ts

* merger conflict resolution

* set api key according to environment (#2422)

* Change clickable, fortmatic supportedChains, enum DefaultInjected

* Change CW deeplink

* Testing walletLink on mobile
fairlighteth pushed a commit that referenced this pull request Mar 2, 2022
@henrypalacios
Use previously supported wallets (#2166)
ffbbda9 
* testing

* Enable Coinbase Wallet (wallet-link) (#2353)

* enable walletlink and upgrade package

* Limiting the supported networks for CoinbaseWallet

* Adding setDefaultInjected provider

* Enable Formatic wallet (#2388)

* Activate formatic wallet

* Adding reconnect Uninjected Provider

* Adding connecting web3Status after refresh

* Upgrade fortmatic to v2.2.1

* Mute unnecessary Overlay useEffect

* Adding new API 🔑

* Adding custom src/custom/connectors/Fortmaitc.ts

* merger conflict resolution

* set api key according to environment (#2422)

* Change clickable, fortmatic supportedChains, enum DefaultInjected

* Change CW deeplink

* Testing walletLink on mobile

(cherry picked from commit e88eb5e)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alfetopito alfetopito alfetopito approved these changes

@elena-zh elena-zh elena-zh approved these changes

@anxolin anxolin Awaiting requested review from anxolin

Assignees

@henrypalacios henrypalacios

Labels
app:CowSwap CowSwap app Protofire Handled by Protofire development team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[wallet] Enable Wallet Link and coinbase Link
3 participants
@henrypalacios @elena-zh @alfetopito