Skip to content
This repository was archived by the owner on Nov 10, 2023. It is now read-only.

Add WalletConnect safe-app#1364

Merged
dasanra merged 6 commits intodevelopmentfrom
issue-1363
Sep 23, 2020
Merged

Add WalletConnect safe-app#1364
dasanra merged 6 commits intodevelopmentfrom
issue-1363

Conversation

@nicosampler
Copy link
Contributor

Closes #1363

@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link

github-actions bot commented Sep 16, 2020

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@ghost
Copy link

ghost commented Sep 16, 2020

Travis automatic deployment:
https://pr1364--safereact.review.gnosisdev.com/app

@tschubotz
Copy link
Contributor

tschubotz commented Sep 16, 2020

I think the app should be called "WalletConnect", not "Safe connect"

From the designs:
image

(https://projects.invisionapp.com/share/T4XF6KSQX7M#/screens/429167947_Apps_-Safe_Connect-_1272px)

@nicosampler
Copy link
Contributor Author

Good catch, I will change it.

@nicosampler nicosampler changed the title Add safe-connect safe-app Add WalletConnect safe-app Sep 16, 2020
@tschubotz
Copy link
Contributor

tschubotz commented Sep 16, 2020

@nicosampler
Do you know of a Rinkeby dapp that I can use to test this? https://example.walletconnect.org somehow doesn't work for me, i.e. I cannot trigger a tx.

Also: When I connect with my Rinekby Metamask to https://app.uniswap.org, it works like a charm. But when I connect via WalletConnect to my RinkebySafe, it shows "Wrong network". Do you have any idea why that is? It's probably on the Uniswap side, but I was wondering if you have any idea :)

@nicosampler
Copy link
Contributor Author

I'm using https://mintr.synthetix.io/

@tschubotz
Copy link
Contributor

I'm using https://mintr.synthetix.io/

How do I get Rinkeby SNX?

@ghost
Copy link

ghost commented Sep 16, 2020

Travis automatic deployment:
https://pr1364--safereact.review.gnosisdev.com/app

@nicosampler
Copy link
Contributor Author

@tschubotz
use this contract address 0x3cA27cEEDA9A200f50C3Cf3802C6c60d43867977 in tx-builder and execute method mint, it will provide you with 200 snx.

After that, you can go to Mintr app and mint some sUSDC. Note that the UI in Mintr will get stuck in "awaiting for approval" but if you pay attention at the list in the left-bottom corner, you will see the balances are updated (sUDC) once you have signed the TX in the safe.

@tschubotz
Copy link
Contributor

use this contract address 0x3cA27cEEDA9A200f50C3Cf3802C6c60d43867977 in tx-builder and execute method mint, it will provide you with 200 snx.

Hmm, this always fails for me, even with Metamask 🤔
https://rinkeby.etherscan.io/tx/0xe055d55339a78fe2680c6ab1f4b87dcbfcf2c2aee5e0ec3e15cc97775631c417

@nicosampler
Copy link
Contributor Author

nicosampler commented Sep 16, 2020

@nicosampler
Copy link
Contributor Author

I can send you some tokens if you give me an address.

@tschubotz
Copy link
Contributor

tschubotz commented Sep 17, 2020

I can send you some tokens if you give me an address.

That would be great! Here's my RInkeby Safe: https://rinkeby.etherscan.io/address/0xae3c91c89153deac332ab7bbd167164978638c30

I'll try to figure out why the tx fails in the meantime.

EDIT: Figured it out, have SNX now :)

@nicosampler
Copy link
Contributor Author

great, now you have 50 more =D. https://rinkeby.etherscan.io/tx/0xe6f18dae561b48e29481cfdb7f7cd4de547cfd44bfac455bdfd8b3aa32ca4db0

@tschubotz
Copy link
Contributor

there are a couple of typos in the description
image

  • In 3: "Paste" is missing the "e"
  • In 5: "view" -> "via"
  • In the link: "to user" -> "to use"

For reference here the link to the invision prototype with correct texts: https://projects.invisionapp.com/share/T4XF6KSQX7M#/screens/429167947_Apps_-Safe_Connect-_1272px

@nicosampler
Copy link
Contributor Author

@tschubotz what was the problem with SNX? @francovenica is having the same issue

@nicosampler
Copy link
Contributor Author

nicosampler commented Sep 18, 2020

@tschubotz, @francovenica and I are having the same issue with Uniswap and Sablier. When we connect the safe with the dapp it shows "wrong network", I been debugging and we are passing the proper chainId for rinkeby = 4.
For Mintr it works fine so that makes me think that it's a problem with these dapps. I suggest to merge it and then test it on mainnet.

Typos will be fixed in safe-apps repo. This PR only adds the url for the safe-app.

@ghost
Copy link

ghost commented Sep 18, 2020

Travis automatic deployment:
https://pr1364--safereact.review.gnosisdev.com/app

@dasanra
Copy link
Collaborator

dasanra commented Sep 18, 2020

@nicosampler I was having also that network error issue with Sablier. Didn't had time to do a further check

@nicosampler
Copy link
Contributor Author

It's happening only on Rinkeby, on Mainnet works fine.
As Mintr works fine in both network I guess is something related with Sablier/Rinkeby.
I debugged the source code and we are sending the correct network.

@nicosampler nicosampler mentioned this pull request Sep 18, 2020
@tschubotz
Copy link
Contributor

This Rinkeby issue on e.g. Uniswap is happening for me also when I use Metamask mobile on Rinkeby with Uniswap.
So it seems to me like it's indeed an issue on the dapp's side.

Still, weird that Metamask extension works on Rinkeby with Uniswap. Would be interesting to know why that is. Here's the uniswap code if anyone wants to take a look: https://github.com/Uniswap/uniswap-interface/

@mmv08
Copy link
Contributor

mmv08 commented Sep 21, 2020

@tschubotz from the source code it seems like uniswap walletconnect's connector supports only mainnet:
https://github.com/Uniswap/uniswap-interface/blob/master/src/connectors/index.ts#L34

injected providers support multiple networks:
https://github.com/Uniswap/uniswap-interface/blob/master/src/connectors/index.ts#L30

@tschubotz
Copy link
Contributor

@tschubotz from the source code it seems like uniswap walletconnect's connector supports only mainnet:
https://github.com/Uniswap/uniswap-interface/blob/master/src/connectors/index.ts#L34

injected providers support multiple networks:
https://github.com/Uniswap/uniswap-interface/blob/master/src/connectors/index.ts#L30

That explains it then, thanks for checking! :)

@ghost
Copy link

ghost commented Sep 21, 2020

Travis automatic deployment:
https://pr1364--safereact.review.gnosisdev.com/app

@ghost
Copy link

ghost commented Sep 23, 2020

Travis automatic deployment:
https://pr1364--safereact.review.gnosisdev.com/app

@francovenica
Copy link
Contributor

Tried again today in this PR adding the app once again
Tried in rinkeby with https://mintr.synthetix.io/ since it works with the rinkeby network

The connection to the app worked just fine
I tried to mint tokens, but at first the tx wasn't showing up in the safe to sign.
Tried again after a while and the tx showup on itself, so it got "stuck" in mintr for a while until it trigger again it seems.

Tried one more time and this time it worked right away, I was able to sign and get the tokens I minted:
image.png

It works fine, but since we cannot test every dapp out there with a WC then we have to keep an eye on user feedback.

@francovenica francovenica self-requested a review September 23, 2020 16:09
@ghost
Copy link

ghost commented Sep 23, 2020

Travis automatic deployment:
https://pr1364--safereact.review.gnosisdev.com/app

@dasanra dasanra merged commit 32547a8 into development Sep 23, 2020
@dasanra dasanra deleted the issue-1363 branch September 23, 2020 18:34
@github-actions github-actions bot locked and limited conversation to collaborators Sep 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add safe-connect app to safe-react app list

5 participants