Developer-DAO / web3-ui

A React UI library for Web3
https://web3-ui.vercel.app/
MIT License
763 stars 152 forks source link

Chain Id Formatted Incorrectly When Changing Networks #273

Closed 0xDerked closed 2 years ago

0xDerked commented 2 years ago

When changing networks/blockchains the Web3 context incorrectly updates the Chain ID from hexadecimal. For example, the localhost chain Id is stored as 1337 in decimal, but 0x539 in hex. The current formatting for 'chainChanged' just rips the 0x off the front instead of a conversion to decimal.

Example: If you are on mainnet and switch to localhost, the chainId in the Web3 context will update to 539 instead of 1337 and there will be a mismatch between the network and chainId and correctNetwork will be false.

In the 'accountsChanged' listener right below it, the chainId is set from the ethers provider and seems to have no issues.

See screenshots below. I'm suggesting using the method marked by red 2 in the listener marked by the red 1. Apologies for any confusion in the explanation.

Once I can get my environment set up properly I'd be happy to do this. Thanks :).

chainIdProps web3ui issue

Dhaiwat10 commented 2 years ago

@Derked is this something that happens with localhost in particular? I personally haven't had problems with other networks

0xDerked commented 2 years ago

@Dhaiwat10 This will be a problem on any chain where the hex conversion to decimal is not equivalent to ripping the 0x off the front. For example since Ethereum mainnet is chain ID 1 and Rinkeby is chain id 4, you won't have any issues since 1 is 0x1 in hex and 4 is 0x4 in hex.

However, for something like Moonriver which is chain ID 1285 in decimal, the hex equivalent is 0x505. When switching to Moonriver you will get a chain ID of 505 instead of 1285. For localhost, the chain ID is 1337 in decimal or 0x539 in hex. Same problem.

Dhaiwat10 commented 2 years ago

@Derked Ahh I see. This is a critical bug. Thanks for catching this

Dhaiwat10 commented 2 years ago

@Derked do you want to work on this one? I can help you get your local environment all setup if needed

0xDerked commented 2 years ago

@Dhaiwat10 Yea I'm happy to do it if I can get WSL to behave. I'll connect with you to spin it up.

shamoilarsi commented 2 years ago

hey @Dhaiwat10 is this issue being worked on? I'd like to take a look as well.

replacing +newChainId.split('0x')[1]; with +newChainId should be a quick fix?

> const chain = "0x89"
> const formattedChain = +chain;
> formattedChain // 137
Dhaiwat10 commented 2 years ago

@shamoilarsi nope, I can assign it to you if you want to work on it

shamoilarsi commented 2 years ago

@Dhaiwat10 yup I'd like to take it. how does my approach sound here?

Dhaiwat10 commented 2 years ago

@shamoilarsi it looks like it could work but I think you'll only find out once you test it 😅