Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Now the default behaviour for switching the chain is actually to try to add it, if it already exists, we're getting a prompt to switch to it. #1250

Merged
merged 8 commits into from
Dec 3, 2024

Conversation

kantagara
Copy link
Contributor

@kantagara kantagara commented Nov 28, 2024

This solves a HUGE UX issue we have with our current SDK. With this PR users are getting the nice prompt to add and or/ switch to the chain that's installed on their wallets.
@juans-chainsafe to test: change the network in project settings to OP Sepolia testnet or any other you prefer.
Make sure you don't have that network in your wallet.
Closes #1231

…to add it, if it already exists, we're getting a prompt to switch to it.
@kantagara kantagara added ready-to-merge Ready to Merge PR - this'll trigger required checks documented Apply on Pull Request after documentation to allow merge and removed ready-to-merge Ready to Merge PR - this'll trigger required checks labels Nov 28, 2024
Copy link
Contributor

@juans-chainsafe juans-chainsafe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying in iOS and is not working, it tries to switch the network and add it, but I get a error:

[Web3] Failed to add and switch to the network: {"code":-32603,"message":"Request for method 'eth_chainId on https://goerli.optimism.io failed","data":{"networkErr":{}}}
ChainSafe.Gaming.Evm.Unity.<ActionWrapper>d__13:MoveNext()
UnityEngine.SetupCoroutine:InvokeMoveNext(IEnumerator, IntPtr)
ChainSafe.Gaming.Evm.Unity.Dispatcher:Update()

It switched to Metamask to add the network (I guess?) but nothing happen there, when I switched back to the Unity app, I got the error.

Steps

  1. Added Optimism Goerli network (because I don't have it in Metamask)
    Screenshot 2024-11-28 at 12 37 05 PM

  2. Build in iOS

  3. Connect with Reown and Metamask

  4. Sign message in Metamask and go back to Unity app

  5. Message of "Switching network" appears and redirects me to Metamask

  6. Nothing happens in Metamask, so I switch back to the app

Copy link
Contributor

@juans-chainsafe juans-chainsafe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, Optimism Goerli is deprecated so is expected to fail.

I retried with Avalanche Fuji and is working! It added the network to my Metamask account and then switched to that network.

I also tried #1201 and is working, so we can close that issue after this gets merged.

Copy link
Member

@creeppak creeppak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement for Reown! Left a couple of questions and suggestions.

Copy link
Member

@creeppak creeppak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍👍👍

@kantagara kantagara added ready-to-merge Ready to Merge PR - this'll trigger required checks and removed ready-to-merge Ready to Merge PR - this'll trigger required checks labels Dec 3, 2024
@kantagara kantagara merged commit 0d5ec26 into dev Dec 3, 2024
7 checks passed
@kantagara kantagara deleted the nikola/add-chain-if-doesnt-exist-reown branch December 3, 2024 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documented Apply on Pull Request after documentation to allow merge ready-to-merge Ready to Merge PR - this'll trigger required checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants