search

celo-org / react-celo

Connect your react dApp to Celo
https://react-celo.vercel.app
MIT License
59 stars 43 forks source link

Instabilities with WalletConnect v2, consider supporting v1 instead #53

Closed jeanregisser closed 2 years ago

jeanregisser commented 2 years ago

There has been a couple of issues with WalletConnect v2 over the past few months.

Latest in date: https://github.com/WalletConnect/walletconnect-monorepo/issues/625

As a result, use-contractkit has been providing a broken WalletConnect experience to DApps and Wallets using it for some time.

Because of these instabilities, for Valora we've decided to disable WalletConnect v2 support until it is considered stable.

At this point I think it would be good to add support for WalletConnect v1 instead in use-contractkit.

Of course my perspective is biased toward Valora and its users, but it seems given all issues we've seen now with v2, we should do what's best for our users.

What do you think?

tunjayhuseynov commented 2 years ago

If it works it is ok, better than the current situation

aslawson commented 2 years ago

@jeanregisser why did you disabled V2 prior to an existing alternative for Celo native dapps integrations? It should not have disrupted existing V1 use cases and disabling will provide the same failed experience as WCV2 being down.

jeanregisser commented 2 years ago

We had no other choice. WalletConnect/walletconnect-monorepo#625 is a show stopper for Valora. It makes it freeze and crash when starting the app. Only way to recover is to reinstall the app and not scan WC v2 QR codes.

It's also a show stopper for dapps, eventually it makes them crash too as documented in the issue.

WC v1 and DAppKit work well enough and are recommended until WC v2 is stable.

aslawson commented 2 years ago

I think it makes sense to disable while we iron out these kinks. We believe it should be stable with this next use-contractkit release. What is are the conditions you are looking for re-enabling?

DAppKit work well enough

Compatibility isn't ubiquitous. We moved to WC to fix this and plan to deprecate DAppkit, but can hold off until Valora transitions. But we won't be able to respond to reported DAppkit issues.

Screen Shot 2021-10-20 at 11 42 06 AM

l

jeanregisser commented 2 years ago

I think it makes sense to disable while we iron out these kinks. We believe it should be stable with this next use-contractkit release. What is are the conditions you are looking for re-enabling?

Unfortunately the issue will remain until WalletConnect/walletconnect-monorepo#625 is fixed and use-contractkit depends on an updated fixed version.

Valora will look to re-enable WC v2 when it's out of beta and our testing confirms it's truly stable.

[DAppKit] Compatibility isn't ubiquitous. We moved to WC to fix this and plan to deprecate DAppkit, but can hold off until Valora transitions. But we won't be able to respond to reported DAppkit issues.

True, DAppKit has some issues, best option today is WC v1 in my opinion.

aslawson commented 2 years ago

I see this issue. Is this the blocker? https://github.com/WalletConnect/walletconnect-monorepo/issues/625

jeanregisser commented 2 years ago

Yes

aslawson commented 2 years ago

Ok, we are looking to do some refactoring to have a config file for wallets to add their data (likely next sprint in 2 weeks). This opens up the possibility for specifying a wallet specific WC implementation (v1 vs v2). Will keep you up to date.

dckesler commented 2 years ago

We added walletconnect v1 support with use-contractkit v2.