Use previously supported wallets #2166

henrypalacios · 2022-01-17T00:15:34Z

Summary

Closes #2165

Proposal:
Try to enable the wallets previously supported by Uniswap and verify that they work.

Coinbase Wallet, Browser Extension / Mobile App Enable Coinbase Wallet (wallet-link) #2353
Formatic, Account-based web., Enable Formatic wallet #2388 Set up Fortmatic wallet environment #2422

Detected problems along the way! 🕵️ :

Fortmatic: the app does not show an error when cancel a transition signing request #2413
⚠️ (issue on hold!) Portis, Account-based web does not work well in Firefox or Brave. Enable Portis wallet #2404

github-actions · 2022-01-17T00:26:46Z

🔭 GP Swap: CoW Protocol v2 Swap UI

henrypalacios · 2022-01-17T15:53:54Z

After enabling the 3 filtered wallets I tried to test each of them and found the following:

CoinbaseWallet: 👇 I'm Working on the first one.

(Web extension/Mobile) ~~When it has an unsupported wallet the application throws an error.~~
(Web extension) ~~When switching between MM and CoinbaseWallet, CoinbaseW seems to overlap MM. And when reloading the wallet information disappears and then it become difficult to reconnect to coinbaseW.~~

It is a very strange behavior that I wanted to know if @elena-zh has any experience about it.

henrypalacios · 2022-01-17T15:56:08Z

Formatic:

This one seems to work fine, however it is only available on Mainnet, I guess this PR will be limited to there @alfetopito?
We need to register the DApp for cowswap.

Portis:

I have not been able to test it, I think is necessary to register the DApp ID for cowswap. Can you help me with that @anxolin?

elena-zh · 2022-01-18T10:52:27Z

After enabling the 3 filtered wallets I tried to test each of them and found the following:

CoinbaseWallet: 👇 I'm Working on the first one.

(Web extension/Mobile) ~~When it has an unsupported wallet the application throws an error.~~

(Web extension) When switching between MM and CoinbaseWallet, CoinbaseW seems to overlap MM. And when reloading the wallet information disappears and then it become difficult to reconnect to coinbaseW.

It is a very strange behavior that I wanted to know if @elena-zh has any experience about it.

Hey @henrypalacios ,
As for the Coinbase wallet mobile, we have an issue on the board #1950 . It is the only thing I could help you with.
As for the injected browser wallet, no, unfortunately I did not tested the app integration with this wallet and I can't help you in providing some details about it 😞

elena-zh · 2022-01-18T11:22:39Z

@henrypalacios , as for the not showing Connect button, I assume the issue might be connected with this behavior: #1187 : I noticed, that now the Swap form appears, but there is no 'connect' button for the described cases

* enable walletlink and upgrade package * Limiting the supported networks for CoinbaseWallet * Adding setDefaultInjected provider

github-actions · 2022-02-14T11:04:51Z

CLA Assistant Lite All Contributors have signed the CLA.

* 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

alfetopito

Both working great!

W3stside · 2022-02-18T12:20:06Z

src/custom/components/Web3Status/Web3StatusMod.tsx

+    `}
+
+  ${({ clickable, pending }) =>
+    clickable !== false &&


if this prop isn't fed clickable explicitly (it's an optional prop with no default) it will still allow clicking, i.e undefined !== false // true

Thank you for such detailed information.

I have moved to explicitly clickDisabled state when disabling.

W3stside · 2022-02-18T12:25:19Z

src/custom/components/Web3Status/Web3StatusMod.tsx

@@ -230,6 +243,17 @@ export function Web3StatusInner({
        <Text>{error instanceof UnsupportedChainIdError ? <Trans>Wrong Network</Trans> : <Trans>Error</Trans>}</Text>
      </Web3StatusError>
    )
+  } else if (thereWasAProvider) {
+    return (
+      <Web3StatusConnected pending={true} clickable={false}>


ubernit: can just pass pending like so:

Suggested change

<Web3StatusConnected pending={true} clickable={false}>

<Web3StatusConnected pending clickable={false}>

W3stside · 2022-02-18T12:26:22Z

src/custom/connectors/Fortmatic.ts

+
+export const OVERLAY_READY = 'OVERLAY_READY'
+
+type FormaticSupportedChains = 1 | 3 | 4 | 42


we should only have chains we support here, should use SupportedChainId enum

W3stside · 2022-02-18T12:26:34Z

src/custom/connectors/Fortmatic.ts

+
+type FormaticSupportedChains = 1 | 3 | 4 | 42
+
+const CHAIN_ID_NETWORK_ARGUMENT: { readonly [chainId in FormaticSupportedChains]: string | undefined } = {


same here probably

W3stside · 2022-02-18T12:27:37Z

src/custom/connectors/index.ts

@@ -4,7 +4,7 @@ import { WalletConnectConnector } from '@web3-react/walletconnect-connector'
 import { WalletLinkConnector } from '@web3-react/walletlink-connector'
 import { PortisConnector } from '@web3-react/portis-connector'

-import { FortmaticConnector } from 'connectors/Fortmatic'
+import { FortmaticConnector, getFortmaticApiKey } from 'custom/connectors/Fortmatic'


shouldn't need custom in the import

Suggested change

import { FortmaticConnector, getFortmaticApiKey } from 'custom/connectors/Fortmatic'

import { FortmaticConnector, getFortmaticApiKey } from 'connectors/Fortmatic'

W3stside · 2022-02-18T12:30:26Z

src/custom/hooks/web3.ts

-      } else {
-        if (isMobile && window.ethereum) {
+  const connectInjected = useCallback(
+    (providerName = 'Metamask') => {


can you not use the WalletProvider.METAMASK enum here? (or does it not exist for some reason?)

It does not exist in WalletProvider because it assumes that the INJECTED provider is Metamask, I have created an enum to distinguish between the injected ones.

W3stside · 2022-02-18T12:31:43Z

src/custom/hooks/web3.ts

@@ -84,3 +97,23 @@ export function useEagerConnect() {

  return tried
 }
+
+export function setDefaultInjected(providerName: 'MetaMask' | 'CoinbaseWallet') {


nit: use enum WalletProvider enum here

Accomplished...

elena-zh · 2022-02-21T13:55:54Z

Hey @henrypalacios , I have faced another issue with Coinbase wallet:

I do a hard refresh of the page OR disconnect a wallet inside the Mobile Coinbase app
the app goes to the 'Connecting' state
I close Coinbase connection window
---> The app hangs in the 'Connecting...' state (until I reload the page.
See the video: https://watch.screencastify.com/v/aeccGsVrwYL3zbXdBlDb

So here I would expected to connect to stop connection process with the Coinbase wallet.

Anyways, it is a low priority. So let me know please If I need to create a separate issue for this.

elena-zh · 2022-02-21T14:13:57Z

This issue is an important to fix: when press on the 'Connect Coinbase wallet' from mobile view, the app navigates to the Coinbase app browser and opens Uniswap app instead of the CowSwap.

See the video:

uni.mp4

elena-zh · 2022-02-21T14:35:41Z

When I have enabled 2 browser extentions (MM and Coinbase), connect a wallet to the MM and try to confirm a wrap/unwrap/approve token transaction, I see Coinbase wallet pop-up with an error there:

However, I can sign Buy/Sell orders transactions (again, I see Coinbase wallet pop-up instead of the MM)

elena-zh · 2022-02-21T15:03:11Z

Fortmatic works fine except this issue: I can't call keyboard in order to connect to the wallet when press on the email field in the connection modal

fortm.mp4

The only way to open the keyboard is to long press on the field, then tap it again (takes a lot of time to understand and find out how to manage this).

elena-zh · 2022-02-22T15:35:21Z

This issue is an important to fix: when press on the 'Connect Coinbase wallet' from mobile view, the app navigates to the Coinbase app browser and opens Uniswap app instead of the CowSwap.

See the video:

uni.mp4

The issue is fixed: the app opens https://cowswap.exchange/#/swap instead of the Uniswap now.
However, I'd propose to open an appropriate environment in the CB wallet according to the one a user is currently working on:

BARN - https://barn.cowswap.exchange/#/swap
Stage - https://cowswap.staging.gnosisdev.com/#/swap
Dev/PRs - https://cowswap.dev.gnosisdev.com/#/swap
Prod - https://cowswap.exchange/#/swap

alfetopito · 2022-02-23T18:23:46Z

src/custom/constants/index.ts

@@ -25,6 +25,9 @@ export const APP_DATA_HASH = getAppDataHash()
 export const PRODUCTION_URL = 'cowswap.exchange'
 export const BARN_URL = `barn.${PRODUCTION_URL}`

+// Change the generated CoinbaseWallet link by going to the DApp on mobile via deeplink
+SUPPORTED_WALLETS_UNISWAP.COINBASE_LINK.href = 'https://go.cb-w.com/o0KpAwewQnb'


How did you generate the link?

The only place I found now was within CoinbaseWallet itself 👇

Which is why it has become a challenge to comply with the following.

"... I'd propose to open an appropriate environment in the CB wallet according to the one a user is currently working on:
BARN - https://barn.cowswap.exchange/#/swap
Stage - https://cowswap.staging.gnosisdev.com/#/swap
Dev/PRs - https://cowswap.dev.gnosisdev.com/#/swap
Prod - https://cowswap.exchange/#/swap

I have tried to remove the COINBASE_LINK

If it's not trivial to update the link, let's stick to PROD and move on.

alfetopito · 2022-03-01T17:02:47Z

As discussed on the meeting @elena-zh those 2 issues:

Coinbase conflicting with metamask
Fortmatic wallet not showing the keyboard

Are out of the scope of this change.
I'll go ahead and merge as is to get this included in the next release.

The issue 1 can be resolved by having only 1 injected wallet enabled at a time.

The issue 2 sounds more like up to the wallet than anything we can do on our side.
You could validate that by trying it out on uniswap.

Still, please open issues for them in the backlog, although I don't think we'll prioritize them any time soon.

henrypalacios added app:CowSwap CowSwap app Protofire Handled by Protofire development team labels Jan 17, 2022

henrypalacios self-assigned this Jan 17, 2022

This comment has been minimized.

Sign in to view

henrypalacios force-pushed the 2165-adding-wallet-supports branch from c442fdb to 891e11f Compare January 17, 2022 23:14

henrypalacios changed the title ~~Mute disabled wallets~~ Do not mute previously supported wallets Jan 18, 2022

henrypalacios force-pushed the 2165-adding-wallet-supports branch 5 times, most recently from e643407 to 8ff122b Compare January 22, 2022 00:49

henrypalacios closed this Jan 28, 2022

henrypalacios force-pushed the 2165-adding-wallet-supports branch from 0c389f3 to 7aa7be5 Compare January 28, 2022 18:47

testing

5c25d5b

henrypalacios reopened this Jan 29, 2022

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

2f27882

* enable walletlink and upgrade package * Limiting the supported networks for CoinbaseWallet * Adding setDefaultInjected provider

henrypalacios changed the title ~~Do not mute previously supported wallets~~ Use previously supported wallets Feb 14, 2022

henrypalacios mentioned this pull request Feb 14, 2022

Set up Fortmatic wallet environment #2422

Merged

henrypalacios marked this pull request as ready for review February 15, 2022 13:55

set api key according to environment (#2422)

b7e4cf3

henrypalacios requested review from ramirotw and alfetopito February 16, 2022 13:24

alfetopito approved these changes Feb 17, 2022

View reviewed changes

ramirotw approved these changes Feb 17, 2022

View reviewed changes

W3stside reviewed Feb 18, 2022

View reviewed changes

Change clickable, fortmatic supportedChains, enum DefaultInjected

9360241

henrypalacios requested a review from W3stside February 18, 2022 22:30

Change CW deeplink

44f489a

henrypalacios force-pushed the 2165-adding-wallet-supports branch from 4d1b6bc to 44f489a Compare February 22, 2022 14:56

Testing walletLink on mobile

fed5510

alfetopito reviewed Feb 23, 2022

View reviewed changes

alfetopito merged commit e88eb5e into develop Mar 1, 2022

alfetopito deleted the 2165-adding-wallet-supports branch March 1, 2022 17:03

github-actions bot locked and limited conversation to collaborators Mar 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use previously supported wallets #2166

Use previously supported wallets #2166

henrypalacios commented Jan 17, 2022 •

edited

Loading

github-actions bot commented Jan 17, 2022

henrypalacios commented Jan 17, 2022 •

edited

Loading

henrypalacios commented Jan 17, 2022 •

edited

Loading

This comment has been minimized.

elena-zh commented Jan 18, 2022

elena-zh commented Jan 18, 2022

github-actions bot commented Feb 14, 2022

alfetopito left a comment

W3stside Feb 18, 2022

henrypalacios Feb 18, 2022

W3stside Feb 18, 2022

W3stside Feb 18, 2022

W3stside Feb 18, 2022

henrypalacios Feb 18, 2022

W3stside Feb 18, 2022

W3stside Feb 18, 2022

henrypalacios Feb 18, 2022 •

edited

Loading

W3stside Feb 18, 2022

henrypalacios Feb 18, 2022

elena-zh commented Feb 21, 2022 •

edited

Loading

elena-zh commented Feb 21, 2022

elena-zh commented Feb 21, 2022

elena-zh commented Feb 21, 2022

elena-zh commented Feb 22, 2022

alfetopito Feb 23, 2022

henrypalacios Feb 23, 2022

alfetopito Feb 23, 2022

alfetopito commented Mar 1, 2022

	<Web3StatusConnected pending={true} clickable={false}>
	<Web3StatusConnected pending clickable={false}>


		export const OVERLAY_READY = 'OVERLAY_READY'

		type FormaticSupportedChains = 1 \| 3 \| 4 \| 42


		type FormaticSupportedChains = 1 \| 3 \| 4 \| 42

		const CHAIN_ID_NETWORK_ARGUMENT: { readonly [chainId in FormaticSupportedChains]: string \| undefined } = {

	import { FortmaticConnector, getFortmaticApiKey } from 'custom/connectors/Fortmatic'
	import { FortmaticConnector, getFortmaticApiKey } from 'connectors/Fortmatic'

Use previously supported wallets #2166

Use previously supported wallets #2166

Conversation

henrypalacios commented Jan 17, 2022 • edited Loading

Summary

github-actions bot commented Jan 17, 2022

henrypalacios commented Jan 17, 2022 • edited Loading

henrypalacios commented Jan 17, 2022 • edited Loading

This comment has been minimized.

elena-zh commented Jan 18, 2022

elena-zh commented Jan 18, 2022

github-actions bot commented Feb 14, 2022

alfetopito left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henrypalacios Feb 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elena-zh commented Feb 21, 2022 • edited Loading

elena-zh commented Feb 21, 2022

elena-zh commented Feb 21, 2022

elena-zh commented Feb 21, 2022

elena-zh commented Feb 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alfetopito commented Mar 1, 2022

henrypalacios commented Jan 17, 2022 •

edited

Loading

henrypalacios commented Jan 17, 2022 •

edited

Loading

henrypalacios commented Jan 17, 2022 •

edited

Loading

henrypalacios Feb 18, 2022 •

edited

Loading

elena-zh commented Feb 21, 2022 •

edited

Loading