-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: add multi-injected-browser provider for eip-6963 #4665
base: main
Are you sure you want to change the base?
feat: add multi-injected-browser provider for eip-6963 #4665
Conversation
hi @jeasonstudio , I saw this PR when I was looking for a way to support eip-6963 in my dapp template. Is this PR means, we'll be able to write code like this? if (window.ethereum) {
if(window.ethereum.providers.length>0) {
// load all wallets here
} else {
const provider:BrowserProvider = new BrowserProvider(window.ethereum);
}
} else {
const metamask:MultiInjectedBrowserProvider= new ethers.MultiInjectedBrowserProvider({ rdns: 'io.metamask' });
// how we will load multiple wallets without specifying their names
} Update: I just saw getEip6963Providers method under MultiInjectedBrowserProvider class. It will do the trick I assume? const allInstalledWallets = new ethers.MultiInjectedBrowserProvider().getEip6963Providers(); |
I like the idea. We definitely need to figure out the best API. It might make sense to make this an extension package for v6, to try some ideas out. I also have another package I’m working on called milkbox to experiment with both EIP-6963 apps and clients. I have a client I’m working on in tandem with it. I’d also love to see how people use these sorts of libraries. :) |
EIP-6963 depends on EIP-1193 (browser provider), and now most wallets support eip6963, so I think it should be more recommended to use MultiInjectedBrowserProvider. As @ricmoo said, the current API may not be the best and needs to be discussed and optimized. |
I think This is the current implementation and the simplest version. Provider discovery and default selection are automatically completed when instantiated. class MultiInjectedBrowserProvider extends BrowserProvider {
// It is up to the developer to actively choose which provider to use
switchEip6963Provider(filter?: { rdns?: string }): Eip6963ProviderDetail;
// Get all injected provider
getEip6963Providers(): Eip6963ProviderDetail[];
} I just checked out the milkbox library, we can implement functionality based on it, but I prefer my approach. EIP6963 is very simple. We only need to focus on how it can be better integrated with ethersjs, rather than how it is implemented itself. For the same reason I'm not using mipd. @ricmoo thank you for your review. |
I agree @jeasonstudio that EIP-6963 could be the recommended usage. I saw this page before finding this PR.
As an example: If I get it right, the following methods in my template will be gone and more clean after EIP-6963 support: So, adding this as a v6 extension package might be better than waiting v7 @ricmoo |
I added a new Provider to support eip6963:
MultiInjectedBrowserProvider
.Some internal changes have been made to
BrowserProvider
, but it does not change its usage.When users need multiple injected providers, they can code like this: