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

Filter duplicate EIP-6963 connectors #4328

Closed

Conversation

EdouardBougon
Copy link
Collaborator

Fix duplicate providers

By default, Wagmi allows the discovery of EIP-6963 compatible extensions. For each extension, it creates an Injected provider using the information “emitted” by each extension: id, name, and logo. Internally, they use their own library, mipd

This behavior is enabled by default but can be disabled with the multiInjectedProviderDiscovery option.
This feature is very useful and is never disabled by applications using Wagmi.

The problem arises when you add “external” providers to the Wagmi configuration, such as MetamaskSDK or coinbaseWalletSDK.

By default, Wagmi will add them to the list of providers it discovered via mipd.

So, in the case of the two providers mentioned above, you may end up with duplicates: an Injected provider representing the installed extension discovered via EIP-6963, and an “external” provider representing the SDK version.

Both will have the same name, logo, etc.

This forces apps to use different strategies either to filter out duplicates from the providers supplied by Wagmi, or to pre-filter the external providers that you want to add to Wagmi.

Example of filter out integration: Uniswap
Example of pre-filter integration: carbondefi.xyz

What I propose is to integrate this logic directly into Wagmi, and by default, not to offer the Injected providers if the user supplies equivalent “external” providers.

This could applies to both Metamask and Coinbase Wallet.

Copy link

changeset-bot bot commented Oct 9, 2024

⚠️ No Changeset found

Latest commit: a5e7927

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Oct 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
wagmi ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 9, 2024 10:55am

@tmm
Copy link
Member

tmm commented Oct 9, 2024

Historically, folks handling this downstream has been our preference – adding a list of IDs to filter or one-off conditionals is something we want to avoid. We will reevaluate and get back on this.

@EdouardBougon
Copy link
Collaborator Author

I totally understand your point about not having IDs or one-off conditionals in the core part.

Another way would be to store the rdns in the custom connector with an optional field.

return createConnector<Provider, Properties>((config) => ({
    id: 'metaMaskSDK',
    name: 'MetaMask',
    rdns: 'io.metamask', // optional field
    type: metaMask.type,
    // ...
}))

And then, read it in the createConfig:

//...
  // check if a mipd provider has a specific connector provided by the user
  const overrideRdns = new Set<string>(
    providedConnectors
      .filter((connector) => connector.rdns)
      .map((connector) => connector.rdns!),
  )
  const hasAlreadyASpecificConnector = (provider: EIP6963ProviderDetail) => {
    if (!preferSpecificConnectorOverInjected) return false
    if (overrideRdns.has(provider.info.rdns)) return true

    return false
  }
//...

--

So each custom connector could decide if they want to override their injected connector or not.

@abretonc7s
Copy link
Collaborator

@tmm. the key issue is that developers may unknowingly end up using the wrong provider, particularly when MetaMask is involved. Both the injected provider and the MetaMask SDK provider have the same name and logo, which creates confusion. This can lead to unintentional behavior, as developers may assume they are using the MetaMask SDK, when in reality, they are connecting via the injected provider.

By integrating this logic directly into Wagmi, we reduce the chances of such mistakes. It ensures that developers connect to MetaMask through the SDK, which provides a more consistent experience. Additionally, SDK connections offer richer features, better support, and future-proof capabilities, making it the preferred option. Developers still have the flexibility to manually handle things differently if needed, but this default behavior would guide them toward the best choice—without them having to overthink or worry about provider differences.

Ultimately, this approach helps both developers and users by minimizing confusion and ensuring a smoother integration.

@tmm
Copy link
Member

tmm commented Oct 16, 2024

Still not 100% sold this is what we want to do, but sketching out a different approach in #4343. Can continue conversation over there.

@tmm tmm closed this Oct 16, 2024
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.

3 participants