Enable Formatic wallet #2388

henrypalacios · 2022-02-03T14:21:57Z

Summary

closes #433
Enable the ability to connect via Formatic

To Test

Go to the Connect to a wallet button and click on the Formatic option.

Background

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

github-actions · 2022-02-03T14:30:41Z

🔭 GP Swap: CoW Protocol v2 Swap UI

alfetopito

It works fine for approval, swap and cancellation 👍
(although their message signing UX is not very useful, you have no idea what you are signing)

But the behaviour on page refresh is broken.

Steps:

Connect with Formatic
Refresh the page.

Upper menu is not loaded:

Refresh the page again.

Upper menu is loaded and you can connect wallet again

Click on connect wallet and pick Formatic

It'll connect without needing to login again

elena-zh · 2022-02-04T16:09:03Z

Hey @henrypalacios , I have done some tests: sell/buy orders work great!
I need to retest wrap/unwrap/approve/claim transactions, but will do it a bit later.
For now, I have faced the same issue @alfetopito mentioned above.
In addition, the app does not show an error message when cancel signing transaction. See the video: https://watch.screencastify.com/v/4oUhXdp7lBovwyTTYTsq
You can see a different behavior for cancelling transactions when connected to Fortmaic and MM.

henrypalacios · 2022-02-04T23:44:39Z

@alfetopito the last change will fix the connection on refresh, maybe we could add a loading notification?

I have created a re-connection of providers that are not injected as it seems that this will be necessary for others as well.
Regarding the UX sign message, I have not found a way to implement what they describe in their documentation.
According to what @elena-zh mentions, I have not been able to catch the fortmatic error in signFn, maybe because I still don't understand how signOrderGp enables the provider internally.

alfetopito · 2022-02-07T20:40:42Z

@alfetopito the last change will fix the connection on refresh, maybe we could add a loading notification?

Sure, if that help UX let's give it a try

1. I have created a re-connection of providers that are not injected as it seems that this will be necessary for others as well.

👍

2. Regarding the UX sign message, I have not found a way to implement what they describe in [their documentation](https://docs.fortmatic.com/web3-integration/user-signing).

I don't think you need to do anything.
It was more of a comment of their UX not displaying anything useful, sorry if it wasn't clear.

3. According to what @elena-zh mentions, I have not been able to catch the fortmatic [error in signFn](https://github.com/gnosis/cowswap/blob/433-enable-formatic/src/custom/utils/signatures.ts#L162), maybe because I still don't understand how [signOrderGp](https://github.com/gnosis/cowswap/blob/433-enable-formatic/src/custom/utils/signatures.ts#L114) enables the provider internally.

Well, yeah, then you'll need to dig into the signing stuff.
When doing the signing, watch out the console for errors from the wallet.
It wouldn't be the first wallet to not return anything at all to not fault of our own.

henrypalacios · 2022-02-08T14:23:40Z

@elena-zh As I mentioned to @alfetopito I have added a notification of connecting when it detects that a provider has connected after refreshing the page.

elena-zh · 2022-02-08T14:29:51Z

@henrypalacios , I was able to test all the rest functionality.
And it works!

However, the issue with cancelling signing I mentioned above is still not fixed.

As for 'connecting..' state, it a great idea! The only thing I can mention that in a certain moment of a 'connecting' state connection modal appears, then is closed. Video: https://watch.screencastify.com/v/mlNWW4B6xVeaqNucalPU
Could you please fix this (not to show connection modal)?

Thanks

alfetopito

Yep, working nicely now 👍

The thing still left to address (besides updating the api keys, but that's a follow up PR) is to handle the wallet "rejection"

As mentioned over slack, the Safe handles it, so we can get "inspired" by whatever they did there :)

henrypalacios · 2022-02-09T15:19:00Z

@elena-zh with the latest change (fortmatic manual update) the rejection wallet should work, what I'm not sure is if everything else is still working, could you test it please? If it works I will go that way.

elena-zh · 2022-02-09T16:26:21Z

Hey @henrypalacios , not I see this error when I try to connect the wallet

I cleared all app data storage, but it did not help.
Could you please take a look at it?

elena-zh · 2022-02-10T10:41:53Z

Now I'm able to connect, but rejection still does not work..

elena-zh · 2022-02-10T15:39:55Z

@henrypalacios , I retested changes under the current PR, and everything looks good to be besides the cancelling orders' signing issue. I have created a separate issue for this #2413 .
So we can stop working under this PR and proceed with the cancelling issue fixing in another one.
Thanks.

alfetopito

As discussed, this is already in a working state.
The other 2 points listed below can be addressed in follow up PRs:

Try to get the signal from the when user closes the signature modal
Use different API keys for different environments

alfetopito · 2022-02-10T16:21:43Z

src/connectors/Fortmatic.ts

+    // TODO implements this
+    // const pollForOverlayReady = new Promise<void>((resolve) => {
+    //   const interval = setInterval(() => {
+    //     if (provider.overlayReady) {
+    //       clearInterval(interval)
+    //       this.emit(OVERLAY_READY)
+    //       resolve()
+    //     }
+    //   }, 200)
+    // })


What was this for? Do you intend to enable it again or is it no needed for the new version?

That's the point, I checked to see if it is necessary. I did not find that it was necessary after much testing. What I did find is that it is responsible for the strange behavior was commented where the modal reappears:

As it is not part of the custom I have not removed it, let me know if you think I should remove it.

Ah, good point, I missed it's the original source file!

Then you should mod it instead and do the same thing (comment it out without deleting it)

elena-zh · 2022-02-11T09:32:10Z

I have retested the latest changes: LGTM (besides an opened issue #2413 )

* 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

* 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)

henrypalacios added 2 commits February 3, 2022 11:09

Activate formatic wallet

ba50a2a

Test with test API key

e286c01

henrypalacios added app:CowSwap CowSwap app Protofire Handled by Protofire development team labels Feb 3, 2022

henrypalacios self-assigned this Feb 3, 2022

alfetopito reviewed Feb 3, 2022

View reviewed changes

Adding reconnect Uninjected Provider

467bdd7

henrypalacios mentioned this pull request Feb 7, 2022

Use previously supported wallets #2166

Merged

2 tasks

Adding connecting web3Status after refresh

7076a2e

alfetopito reviewed Feb 8, 2022

View reviewed changes

Upgrade fortmatic to v2.2.1

b479d46

using test API 🔑

7a126b2

elena-zh mentioned this pull request Feb 10, 2022

Fortmatic: the app does not show an error when cancel a transition signing request #2413

Open

alfetopito approved these changes Feb 10, 2022

View reviewed changes

henrypalacios added 3 commits February 10, 2022 19:22

Remove unnecessary Overlay useEffect

902fa1c

Adding new API 🔑

fe01bd9

Remove unnecessary import

2c2082e

henrypalacios added 2 commits February 11, 2022 15:05

Adding custom Fortmaitc.ts

aeac6ef

Merge branch '2165-adding-wallet-supports' into 433-enable-formatic

bad91ef

merger conflict resolution

8f2e298

henrypalacios merged commit 45499d0 into 2165-adding-wallet-supports Feb 14, 2022

henrypalacios deleted the 433-enable-formatic branch February 14, 2022 16:09

henrypalacios mentioned this pull request Feb 14, 2022

Set up Fortmatic wallet environment #2422

Merged

anxolin mentioned this pull request Apr 2, 2022

Fortmatic: the app does not show an error when cancel a transition signing request cowprotocol/cowswap#274

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Formatic wallet #2388

Enable Formatic wallet #2388

henrypalacios commented Feb 3, 2022 •

edited

Loading

github-actions bot commented Feb 3, 2022

alfetopito left a comment

elena-zh commented Feb 4, 2022

henrypalacios commented Feb 4, 2022

alfetopito commented Feb 7, 2022

henrypalacios commented Feb 8, 2022

elena-zh commented Feb 8, 2022

alfetopito left a comment

henrypalacios commented Feb 9, 2022

elena-zh commented Feb 9, 2022 •

edited

Loading

elena-zh commented Feb 10, 2022

elena-zh commented Feb 10, 2022

alfetopito left a comment

alfetopito Feb 10, 2022

henrypalacios Feb 10, 2022

alfetopito Feb 11, 2022

elena-zh commented Feb 11, 2022

Enable Formatic wallet #2388

Enable Formatic wallet #2388

Conversation

henrypalacios commented Feb 3, 2022 • edited Loading

Summary

To Test

Background

github-actions bot commented Feb 3, 2022

alfetopito left a comment

Choose a reason for hiding this comment

elena-zh commented Feb 4, 2022

henrypalacios commented Feb 4, 2022

alfetopito commented Feb 7, 2022

henrypalacios commented Feb 8, 2022

elena-zh commented Feb 8, 2022

alfetopito left a comment

Choose a reason for hiding this comment

henrypalacios commented Feb 9, 2022

elena-zh commented Feb 9, 2022 • edited Loading

elena-zh commented Feb 10, 2022

elena-zh commented Feb 10, 2022

alfetopito left a comment

Choose a reason for hiding this comment

alfetopito Feb 10, 2022

Choose a reason for hiding this comment

henrypalacios Feb 10, 2022

Choose a reason for hiding this comment

alfetopito Feb 11, 2022

Choose a reason for hiding this comment

elena-zh commented Feb 11, 2022

henrypalacios commented Feb 3, 2022 •

edited

Loading

elena-zh commented Feb 9, 2022 •

edited

Loading