feat: add WalletConnect execution

Sekhmet commented 9 months ago

Summary

Closes: https://github.com/snapshot-labs/sx-ui/issues/386

Looking for approach/design approvals foremost.

How to test

Go to treasury page of this space: http://localhost:8080/#/sep:0xeb8ea538c90d2e009460ed69add7025560e18481/treasury
Go to Uniswap on speolia: https://app.uniswap.org/swap?chain=sepolia
Connect both, create some execution.

Screenshots

bonustrack commented 9 months ago

When I click approve, I can't see any loading within the approve button, it appear full white
I was not able to login on Uniswap Sepolia or Aave (https://staging.aave.com/markets/) for some reason it get stuck when I click approve, I see this error in the console:

index-b727bc3d.js:573 Uncaught (in promise) Error: Non conforming namespaces. approve() namespaces chains don't satisfy required namespaces. Required: eip155:1 Approved: eip155:11155111

For the button icon we should use WalletConnect logo directly, here is the official SVG: https://github.com/WalletConnect/walletconnect-assets/blob/master/Logo/Blue%20(Default)/Logo.svg ideally the icon would use link color (white and black)
We can change the tooltip and modal title to "Connect to apps"
For the URL it looks soso with mono I would instead make it simple like this:

Sekhmet commented 9 months ago

Should work now, I wasn't able to test with Aave because WalletConnect doesn't open on that URL.

bonustrack commented 9 months ago

It work for me now on Uniswap but only to swap ETH to UNI, when I try to wrap ETH to WETH it doesnt trigger a new transaction.

For the flow what would be better is to keep the WalletConnect connection alive regardless of the page you staying on SX, currently if I change page it will break connection. Also when I trigger a new tx on Uniswap (or other app) we should show a modal with "Add transaction" populated with the tx and confirm button, and this even if I'm still on the treasury page. We should redirect user to proposal creation page only if user confirm the new tx.

Otherwise modal title should be "Connect to apps"

Sekhmet commented 9 months ago

For the flow what would be better is to keep the WalletConnect connection alive regardless of the page you staying on SX, currently if I change page it will break connection.

I'm not sure how to handle that UX-wise. Connection needs to be per-space (because we are connecting to app as treasury from specific space). So what happens if we navigate from the space? And if we navigate from it how do we disconnect? If we put the button on topnav it might end up being confusing (as it doesn't look space-specific anymore).

Do you think any specific idea in mind?

bonustrack commented 9 months ago

If I have a connection on space A and I move to another space B then we wont see the connection in WC modal, another session can be started from space B in parallel of the first space A connection. But if you confirm a transaction you get redirected to the editor for the correct space where connection was initiated. Would this works?

bonustrack commented 9 months ago

Looks good! Some others improvements:

We should have the ability to add more than 1 tx, currently adding another one delete previous one.
Instead of showing the modal with "New transaction has been detected" we should show the modal "Add transaction" directly. And if user click "Confirm" it get added in the proposal.
Uniswap app icon doesn't load for me, for some reason it was working previously. I can see Metamask return this image: https://app.uniswap.org/favicon.png and not a .ico image.

Sekhmet commented 9 months ago

@bonustrack Uniswap updated their metadata on WalletConnect and their image they return first no longer resolves.

bonustrack commented 9 months ago

Ok then no worry will probably be fixed at some point

bonustrack commented 8 months ago

Somehow something is wrong with the background, showing white color on scroll see: https://www.loom.com/share/8a6dbcd305c84c108ce6cf857af58fe7

I'm now able to add multiple txs, but it's hard to review what's the tx is about, what I recommend is to show this modal directly when a new tx is triggered:

Sekhmet commented 8 months ago

Somehow something is wrong with the background, showing white color on scroll see: https://www.loom.com/share/8a6dbcd305c84c108ce6cf857af58fe7

This was fixed there: https://github.com/snapshot-labs/sx-ui/pull/858

snapshot-labs / sx-ui