search

MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
12.03k stars 4.91k forks source link

Address error with private network / ganache #6214

Closed tclairet closed 5 years ago

tclairet commented 5 years ago

Describe the bug With a private network or ganache Metamask print an error "No ETH network, set to lowercase" with address with uppercase letter.

To Reproduce Steps to reproduce the behavior:

  1. Start ganache-cli 
    ganache-cli
  2. Connect metamask to "Localhost 8545".
  3. Try to make a transaction to an address with uppercase letter for example 0x581e97b7E8B650230F549C0EDCbe7B569Bda16bc

Screenshots (sorry for french)

Browser details (please complete the following information):

Additional context This was working with previous version of Metamask.

tmashuang commented 5 years ago

I believe this PR, which prevents checksum addresses on non-ETH blockchains. The issue with ganache-cli is that the networkId is set to the initialized time, which is always changing. The quick options would be to either use Ganache App(which defaults to networkId 5777), or set the networkId as an additional argument ganache-cli -i "5777"/ganache-cli -i 5777.

tclairet commented 5 years ago

So if my private network has a networkID different than 1, 3, 4, 42 or 5777 Metamask will no longer support checksummed address ? Is that definitive or just a collateral issue ?

tmashuang commented 5 years ago

It was mostly definitive, I believe for POA(xDai) network that uses non-checksummed address. If more networks implement checksummed address we can look to revert the PR, or just keep adding on to the networkId list as needed.

davidmurdoch commented 5 years ago

The issue described in https://github.com/MetaMask/metamask-extension/issues/5838 was that addresses were displayed with an incorrect checksum for the connected network.

Perhaps the fix could just be to set an option in the Custom RPC dialog to select whether or not the RPC endpoint should require addresses to be ETH checksum validated or not? Or even just use the whitelist to display imported addresses as checksummed (without requiring user-entered addresses to be lowercase for non-whitelisted network ids).

It'd make for a bad user experience if metamask required that users manually checksummed an all lower-case recipient address before sending, yet metamask now requires that users manually convert already (sometimes valid) checksummed addresses to lower case.

Additionally, metamask currently tells the user that the ganache-cli network they are connected to is not an Eth network, which makes ganache-cli look like it is not a compliant eth network, which is a perception we do not want our users to have.

NicolasMassart commented 5 years ago

I have issues seeing the bug fixed. I have latest 6.2.1 version of the extension and tried it on Firefox and Chrome but I still have the error message on a local RPC network. I tried to remove and reinstall extension but it doesn't change the behaviour. Any ideas why ?

NicolasMassart commented 5 years ago

Seems to be fixed with 6.2.2

jonathansmirnoff commented 5 years ago

With this https://github.com/MetaMask/metamask-extension/pull/6280 PR the issue #5838 is opened again. @whymarrh and @frankiebee, do you mind tell us the expected solution you want achieve? We are happy to collaborate. You can review the PR https://github.com/MetaMask/metamask-extension/pull/6001

Also one possibility is to modify the validation to only check if the chain id is RSK (30,31,32) as we suggested in the beginning.