search

clovers-network / clovers-dapp

Discover, collect & trade cryptographic icons
https://clovers.network
55 stars 21 forks source link

Walletconnect login for Gnosis Safe #417

Open pcowgill opened 4 years ago

pcowgill commented 4 years ago

Describe the bug After clicking WalletConnect as the login option, after scanning the QR code with the Gnosis Safe, it still prompts me to connect with MetaMask.

To Reproduce Steps to reproduce the behavior:

  1. Click Sign in on Chrome or Firefox
  2. Click WalletConnect
  3. Scan the QR code with the Gnosis Safe personal edition app on iOS
  4. Appears to have succeeded in Gnosis Safe app, but not the Clovers web dapp.

Expected behavior Web3 provider is initialized properly using WalletConnect and another wallet without prompting me to connect to MetaMask.

Screenshots Not needed

Desktop (please complete the following information):

Additional context n/a

okwme commented 4 years ago

Hi @pcowgill thank you for opening an issue! While wallet connect works with private key wallets like Rainbow, it unfortunately does not with contract based wallets like Gnosis Safe. I've discussed with their team what it would take to successfully integrate contract based wallets to do sign in requests and it's unfortunately not a priority on their roadmap. It might be possible to do ourselves and make a PR but currently also not our priority. I apologize for the inconvenience of not being able to use the Gnosis Safe at this point and hope you understand the current reasons : \

pcowgill commented 4 years ago

@okwme Thanks for getting back to me!

This isn't a priority on WalletConnect's roadmap or Gnosis Safe's roadmap? This is surprising since Gnosis has WalletConnect integration microgrants

pcowgill commented 4 years ago

I do see on the microgrant page though that one of the criteria is that the dapp should not use message signing or it needs to use a workaround like DAOStack Alchemy's. But then again they do say that contract signature integration was supposed to be live back in August.

pcowgill commented 4 years ago

In the Argent WalletConnect integration they support EIP-1271, so if the dapp supports it I think it should work.

okwme commented 4 years ago

I just tried Argent and it's working with no modifications to the Clovers network Dapp. Any idea what's needed to change in clovers to support gnosis at this point?

On Sun, Dec 8, 2019 at 5:57 PM Paul Cowgill notifications@github.com wrote:

In the Argent WalletConnect integration they support EIP-1271, so if the dapp supports it I think it should work.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/clovers-network/clovers-dapp/issues/417?email_source=notifications&email_token=AAHLLVB7AFXJSQW3TRHSRQDQXURODA5CNFSM4JOZRB52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGHDUSY#issuecomment-562969163, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHLLVA7A4WOHPNN3HGWOJLQXURODANCNFSM4JOZRB5Q .

-- billyrennekamp.com

okwme commented 4 years ago

it's actually kind of crazy and i don't understand how it's happening. I just opened a support ticket with them to ask how they do it. I just confirmed that the wallet address (where the ENS is pointed) is a proxy contract. However via walletconnect I was able to sign the login message and my server was able to recover the contract address!

That would mean there's a pk for the contract right?!

pcowgill commented 4 years ago

Oh, cool!!

I haven’t done a deep dive into EIP-1271 just yet, but I would assume the answer to how it all works is in the spec.

pcowgill commented 4 years ago

I suppose we should rename this issue to be specifically about the Gnosis Safe, then.

okwme commented 4 years ago

ja good idea, just for reference i'm using https://github.com/MetaMask/eth-sig-util to verify signatures created at https://github.com/clovers-network/clovers-dapp/blob/master/src/store/actions.js#L541 and verified at https://github.com/clovers-network/clovers-api/blob/master/src/middleware/auth.js#L32