Assign window.ethereum to givenProvider when available

[cgewecke]

Description

#3374

Metamask is rolling out their upgrade to an EIP-1193 compliant provider while preserving web3.currentProvider.sendAsync method for backwards compatibility.

It looks like the original idea in RequestManager/givenProvider was to defer to the injected provider, but the standard's name for this changed at some point in the last 2 years and code here was not updated.

Resources:

• patch for overwrite at aragon/use-wallet
• declaration of intent at Metamask 7710 (Dec 2019)

Questions:

• Is this a breaking change? (Probably not, see comment)
• Can web3 consume the new Metamask provider? (Probably, see comment but this may need more investigation wrt EIP 1193)

Fixes #3374 (sort of...the issue makes a more general point about givenProvider side-effects)

Type of change

• Bug fix (possibly breaking...)

Checklist:

• I have selected the correct base branch.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.
• Any dependent changes have been merged and published in downstream modules.
• I ran npm run dtslint with success and extended the tests and types if necessary.
• I ran npm run test:unit with success and extended the tests if necessary.
• I ran npm run build-all and tested the resulting file/'s from dist folder in a browser.
• I have updated the CHANGELOG.md file in the root folder.
• I have tested my code on the live network.

[coveralls]

Coverage increased (+0.01%) to 89.845% when pulling 5a92c0c on issue/3374 into 9e46628 on 1.x.

[nivida]

Is this a breaking change?

This shouldn't break anything from my point of view.

Can web3 consume the new Metamask provider?

If they provide the mentioned backward compatibility yes. Otherwise, would we have to update the RequestManager and probably also the subscription object to support the EIP-1193. But I would vote to first finalize the EIP and to add the getCapabilitites method we talked about during devcon in Osaka. The getCapabilitites is required for libs to detect the EIP-1193 provider accordingly.

[nivida]

Open discussion EIP-1193: ethereum/EIPs#2319

[cgewecke]

@nivida Thanks!

If they provide the mentioned backward compatibility yes

Dan Finlay says explicitly in Metamask 7710 that:

The sendAsync() method is not going to change or break, so using it is one safe way to avoid these issues.

...so that base seems covered.

Additional background: there's a Metamask blog post from Nov 6 advising developers to the follow this pattern wrt account access:

window.addEventListener('load', async () => {
    // Modern dapp browsers...
    if (window.ethereum) {
        window.web3 = new Web3(ethereum);
        try {
            // Request account access if needed
            await ethereum.enable();
            // Acccounts now exposed
            web3.eth.sendTransaction({/* ... */});
        } catch (error) {
            // User denied account access...
        }
    }
    // Legacy dapp browsers...
    else if (window.web3) {
        window.web3 = new Web3(web3.currentProvider);
        // Acccounts always exposed
        web3.eth.sendTransaction({/* ... */});
    }
    // Non-dapp browsers...
    else {
        console.log('Non-Ethereum browser detected. You should consider trying MetaMask!');
    }
});

[nivida]

FYI: await ethereum.enable(); !== EIP-1193 :)

[cgewecke]

Note: this PR was originally put on hold because the mosaic repo instantiated Web3 for their test utilities as below:

const web3 = new Web3(Web3.givenProvider || 'http://localhost:8545');

and called it like this:

    web3.currentProvider.send({
      method: 'evm_mine',
      jsonrpc: '2.0',
      id: 1337,
    },
    (err) => {
      if (err) {
        return reject(err);
      }

      const newBlockHash = web3.eth.getBlock('latest').hash;

      return resolve(newBlockHash);
    });
  });

However in Buidler, global.ethereum's send method is async. These calls errored with:

Error: Method [object Object] not supported.
    at HttpProvider.send (/home/travis/build/ethereum/web3.js/mosaic-1/node_modules/@nomiclabs/buidler/src/internal/core/providers/http.ts:55:34)
    at wrapper_1.wrapSend (/home/travis/build/ethereum/web3.js/mosaic-1/node_modules/@nomiclabs/buidler/src/internal/core/providers/accounts.ts:219:21)

I've just removed the mosaic fork's Web3 instantiation logic for unrelated reasons and the test now passes, but I think it might still be a case to be aware of.

Buidler's ethereum provider is being revised in for backwards compatibility with sendAsync in buidler PR 608.

[ryanio]

great thanks, good to know.

I'd also like to add and use the new request method from EIP-1193 with fallback to sendAsync, but can do in a separate PR if you think that would be better. I've been relayed that it should be in MetaMask prod sometime next week, available now as a rc build here.

[cgewecke]

I'd also like to add and use the new request method from EIP-1193 with fallback to sendAsync

Sounds good to me!

[ryanio]

great thanks, on second thought I'll do it in a follow-up PR to keep this one targeted for the intended issue.

[ryanio]

LGTM!