Skip to content

feat: implement EIP-6963 support #731

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

Closed

Conversation

Ikari-Shinji-re
Copy link
Member

@Ikari-Shinji-re Ikari-Shinji-re commented May 16, 2024

Summary

To avoid conflicts and overrides with EVM providers, this pull request implements support for EIP-6963. Using this method, we dispatch an event to request data and instances of EVM providers. Wallets that have implemented EIP-6963 respond by dispatching another event, which includes metadata about the providers and references to their provider objects. We listen for these events to obtain the intended provider. Currently, it seems some wallets, such as CLV (Clover Wallet), Math Wallet, Taho, and Halo, do not implement EIP-6963. We can check these providers again later.

To preserve the current architecture of our wallets package, we currently perform this process for every wallet that implements EIP-6963. Later, we can modify our design to dispatch the request event once and display all wallets that respond to our request, regardless of whether we have implemented them or not. For wallets that do not support EIP-6963, we will continue to obtain their instances using the old method (EIP-1193).

https://eips.ethereum.org/EIPS/eip-6963
https://metamask.io/news/developers/how-to-implement-eip-6963-support-in-your-web-3-dapp/

How did you test this change?

Enable all providers modified in this pull request and simultaneously attempt to connect and execute a swap with them. They should function together without conflicts.

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas

@yeager-eren
Copy link
Collaborator

yeager-eren commented Feb 1, 2025

I'm going to close this PR and we need to have another approach to add support for this EIP:

  • Each provider is firing the event separately, it should be called once when page is loaded.
  • A part of instance is coming asynchronously from eip 6963 and rest of that will be come what injected to window (e.g brave getting evm instance from the eip and solana from window). One can be used instantly and one may arrives a bit later. this can introduce unexpected behaviors and also the goal of this task was reducing conflicts between wallets but in fact it may solve it partially.
  • you've turned on isAsyncProvider for all of them, we need to check the flow carefully to make sure this will not break any wallet, we did use the option for WC2 which has different requirements.
  • Testing it can be time consuming because all the affected wallet should be tested. Not sure this changes add that much value. I see it as improvement.

I would suggest to do it gradually where it adds value (e.g. for our most used wallets) and meanwhile think about add a new behavior to get instances asynchronously. It can be integrated in Hub more easily because it has 3 layers (hub, provider, namespace) and we can fire the announce event on top most layer once. it also has action which we can have an action for this behavior and use it in supported wallets.

Just created a task, 2226.

@yeager-eren yeager-eren closed this Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants