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

EIP-1193 injected connectors mixes with wrong connector id's #3606

Closed
1 task done
magiziz opened this issue Feb 16, 2024 · 11 comments
Closed
1 task done

EIP-1193 injected connectors mixes with wrong connector id's #3606

magiziz opened this issue Feb 16, 2024 · 11 comments
Labels
Good First Issue Misc: Good First Issue

Comments

@magiziz
Copy link
Contributor

magiziz commented Feb 16, 2024

Describe the bug

Let's say i have my own connector metadata which includes id, name and provider.

Note: With metamask we'll use window.ethereum.providers, but with other connectors we would just use the normal window.ethereum provider

// Custom connector metadata "id" and "name"
export const customMetaDataConnectors = [
  {
    id: "metamask",
    name: "MetaMask Wallet",
    provider: window.ethereum.providers?.find(isMetaMask),
  },
  {
    id: "rainbow",
    name: "Rainbow Wallet",
    provider: window.ethereum,
  },
  {
    id: "enkrypt",
    name: "Enkrypt Wallet",
    provider: window.ethereum,
  },
  {
    id: "bitski",
    name: "Bitski Wallet",
    provider: window.ethereum,
  },
];

Once we set a metadata we create our own connectors and append them in createConfig:

import { http, createConfig, createConnector } from "wagmi";
import { mainnet, sepolia } from "wagmi/chains";
import { customMetaDataConnectors } from "./utils/createCustomConnector";
import { InjectedParameters } from "wagmi/connectors";
import { injected } from "wagmi/connectors";

export const createCustomConnector = (
  injectedParams: InjectedParameters,
  customMetaData: { id: string; name: string }
) => {
  return createConnector((config) => ({
    ...injected(injectedParams)(config),
    ...{ rkDetails: customMetaData },
  }));
};

export const config = createConfig({
  chains: [mainnet, sepolia],
  connectors: [
    ...customMetaDataConnectors.map((c) =>
      createCustomConnector(
        {
          target() {
            return {
              id: c.id,
              name: c.name,
              provider: c.provider,
            };
          },
        },
        { ...c }
      )
    ),
  ],
  multiInjectedProviderDiscovery: false,
  ssr: true,
  transports: {
    [mainnet.id]: http(),
    [sepolia.id]: http(),
  },
});

With window.ethereum when you disconnect your wallet and refresh it might pick up some random window.ethereum providers like bitski or enkrypt when it was never connected, but with metamask it works fine since we use window.ethereum.providers instead and try to find the right provider.

You can test that by using useAccount hook and get connector?.id which will display the current connected connector.

Link to Minimal Reproducible Example

https://wagmi-v2-injected-reproduction.vercel.app/

Steps To Reproduce

  1. Have metamask wallet and rainbow wallet extension enabled and disconnect them both.
  2. Connect to metamask and disconnect the wallet
  3. Connect to rainbow and disconnect the wallet
  4. Refresh the page and you'll see that the current connector id is not correct

Video:

reproduce-v2.mov

Wagmi Version

2.5.7

Viem Version

2.7.9

TypeScript Version

5.2.2

Check existing issues

@magiziz
Copy link
Contributor Author

magiziz commented Feb 16, 2024

I don't have bitski wallet and enkrypt wallet. Only rainbow and metamask.

@1997roylee
Copy link
Contributor

I cannot run your demo, it throw "ProviderNotFoundError: Provider not found." on console.

@magiziz
Copy link
Contributor Author

magiziz commented Feb 17, 2024

@1997roylee Pushed a change now. Can you try now ?

@1997roylee
Copy link
Contributor

You should add checks for each provider. Pressing 'Disconnect Wallet' won't actually disconnect MetaMask; your wallet is still connected to the current site. the object "window.ethereum" is "alive".

Screenshot 2024-02-22 at 4 45 52 PM

@magiziz
Copy link
Contributor Author

magiziz commented Feb 22, 2024

@1997roylee Yep that's correct MetaMask won't get disconnected since window.ethereum session is still alive. The problem becomes whenever i refresh the page i need some sort of way to see what connector.id is currently alive.

Right now if i have enkrypt, bitski and have disconnected metamask it'll still mark enkrypt as "connected" when i haven't interacted with that connector yet.

I know the logic right now is based on window.ethereum provider, but could be great to get the correct connector.id.

@1997roylee
Copy link
Contributor

@1997roylee Yep that's correct MetaMask won't get disconnected since window.ethereum session is still alive. The problem becomes whenever i refresh the page i need some sort of way to see what connector.id is currently alive.

Right now if i have enkrypt, bitski and have disconnected metamask it'll still mark enkrypt as "connected" when i haven't interacted with that connector yet.

I know the logic right now is based on window.ethereum provider, but could be great to get the correct connector.id.

thats why we hv to implement a logic to validate window.ethereum is rainbow, is enkrypt, is bitski, otherwise we wont know what's wallet u are connecting.

@magiziz
Copy link
Contributor Author

magiziz commented Feb 22, 2024

@1997roylee Got it. This means i would have to create my own connector and check for is rainbow, is enkrypt, is bitski and etc ? Isn't that hard for maintenance to keep it up to date ?

Can't there be a check in wagmi by seeing if rainbow connector.id was connected to window.ethereum, if true connect the window.ethereum wallet and return rainbow as connector.id ?

@magiziz
Copy link
Contributor Author

magiziz commented Apr 26, 2024

@tmm Can you reopen this issue ? Seems like github actions closed it as "not planned"

@jxom jxom reopened this Apr 26, 2024
@jxom
Copy link
Member

jxom commented Apr 26, 2024

silly @github-actions

@tmm
Copy link
Member

tmm commented Jun 5, 2024

Linking up reproduction source code https://github.com/magiziz/wagmi-v2-injected-reproduction/tree/main

Copy link
Contributor

This issue has been locked since it has been closed for more than 14 days.

If you found a concrete bug or regression related to it, please open a new bug report with a reproduction against the latest Wagmi version. If you have any questions or comments you can create a new discussion thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Good First Issue Misc: Good First Issue
Projects
None yet
Development

No branches or pull requests

4 participants