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

chainChanged send a String instead of an integer #899

Closed
xmaysonnave opened this issue Jun 20, 2020 · 6 comments
Closed

chainChanged send a String instead of an integer #899

xmaysonnave opened this issue Jun 20, 2020 · 6 comments
Labels
discussion Questions, feedback and general information.

Comments

@xmaysonnave
Copy link

I'm using ethers 5.X and started to beta test the upcoming Metamask 8.0.0

Lately I focused on https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1193.md and realized the following situation.

Metamask used to send chainId as a decimal in the following snippet but now rather send an hexadecimal string who needs to be parseInt.

MetaMask/metamask-extension#8837

window.ethereum.on('chainChanged', chainId => {
   self.chainChanged(chainId)
})

In my code I use the following to retrieve the chainId

const web3 = new window.ethers.providers.Web3Provider(window.ethereum)
const network = await web3.getNetwork()
const chainId = network.chainId

However in ethers chainId is a number rather than an hexadecimal string.

My motivation here is more to avoid any fragmentations between libraries in my dapp and a wish to report what I noticed. Any thought will be greatly appreciated.

Thanks

@ricmoo
Copy link
Member

ricmoo commented Jun 20, 2020

The goal of Ethers is not to mimic EIP-1193 or any other Provider. The Web3Provider (which will be renamed in the future) is designed to accept a variety of backends (such as window.ethereum or instances of the classes in web3-providers) and provide a consistent output, regardless of what the backend does. If I could go back in time, I would not have called anything in ethers a Provider, as it becomes conflated with a large collection of things various clients expose which are all slightly different all called Provider.

So, ethers v5 will always return a number for chain ID. So, any library which relies on an "ethers Provider", does not need to worry about handling fragmentation amongst backend providers, ethers will de-fragment that for you.

Also, keep in mind, if you want to support a backend which can change networks, you need to specify the network "any".

What you likely want is something like:

// Notice this: ----------------------------------------------------v
const provider = new ethers.providers.Web3Provider(window.ethereum, "any")
const network = await provider.getNetwork()
const chainId = network.chainId

// You can also then use this:
provider.on("network", (network, oldNetwork) => {
    console.log(network.chainId);
});

Without the "any", if the network does not match its original network, calls on it will fail.

For more information see #866 .

Make sense? :)

@ricmoo ricmoo added the discussion Questions, feedback and general information. label Jun 20, 2020
@ricmoo
Copy link
Member

ricmoo commented Jun 20, 2020

(oh! Another note; there will always be at least one "network" event, with the oldNetwork as null, which is emitted before any other Provider call will succeed)

@xmaysonnave
Copy link
Author

Many thanks for this clarification.
It definitely make sense.
It looks interresting, currently I use this from EIP-1193

      this.provider.on('accountsChanged', accounts => {
        self.accountChanged(accounts)
      })
      this.provider.on('chainChanged', chainId => {
        self.chainChanged(chainId)
      })
      this.provider.on('connect', chainId => {
        self.chainChanged(chainId)
      })
      this.provider.on('disconnect', (code, reason) => {
        self.disconnectedFromAllChains(code, reason)
      })
      this.provider.on('message', message => {
        self.providerMessage(message)
      })

Where provider is window.ethereum.

Does It means that I can rely on a ethers Provider instead to track wallet events ?
I quickly checked the doc and didn't find specifics about the ethers provider.on method.
Any pointer, tests samples, etc... would be greatly appreciated. I will do my homework though.

By the way thanks for your new release. Seemless migration.

Thanks

@ricmoo
Copy link
Member

ricmoo commented Jun 20, 2020

This is prolly what you are interested in: https://docs.ethers.io/v5/api/providers/provider/#Provider--events

There is no "connect" or "disconnect" in ethers (yet; likely); more thought needs to be added around the concept that is meaningful to all backends. The "connect" is probably the same as "network" though.

Keep in mind a Provider in ethers is meant to be read-only, the Signer-based events are outside the scope of Provider. A wrapper can be used on a JsonRpcProvider (which Web3 extends) to detect account changes. The conflation of what a Signer and Provider are in ethers with what most other libraries refer to singularly as Provider makes some of these events and ideas hard to resolve.

In the future, the JsonRpcProvider sub-class may add an event equivalent to "accountChanged" (probably "accounts", which would be passed the array of accounts; but also Signer needs an event to be notified its index has possibly changed), but nothing like that exists yet. For now, if you have an EIP-1193 provider, you can use its "accountChanged" event. That part hasn't been un-fragmented yet. Much more thought needs to happen. :(

@xmaysonnave
Copy link
Author

Thanks for the link.
Currently I use two providers.
One read-only without a signer and another one with a signer. built each time a request is done.
That way I don't bother users with an enabled dialog box for read only operations.
I'll definitely monitor and study what you suggest.

@ricmoo
Copy link
Member

ricmoo commented Jun 29, 2020

Exactly how I recommend doing it. I want my Signer to be as far removed from my main Provider as necessary. :)

Closing this now, but feel free to re-open if you have further issues.

Thanks! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Questions, feedback and general information.
Projects
None yet
Development

No branches or pull requests

2 participants