apps icon indicating copy to clipboard operation
apps copied to clipboard

Integrate MetaMask/web3 into polkadot-js interface

Open joelamouche opened this issue 4 years ago • 17 comments

As of now, for an ethereum compatible blockchain, the user has to manually import their MetaMask accounts to the app by exporting the private key and entering it.

This is a little bit of friction that could be avoided if we used the injected web3 object to list accounts and sign transactions.

It would require a lot of code change because it would mean using web3 instead of ui-keyring everywhere (if isEthereum is true or maybe a manual switch).

Maybe there is a way to wrap web3 to have the same interface as keyring.

What do you think @jacogr ?

joelamouche avatar Nov 13 '20 15:11 joelamouche

If it can expose this extension interface, it can work as a normal extension - https://github.com/polkadot-js/extension/blob/master/packages/extension-inject/src/types.ts#L95

A compat layer has actually been done before, see https://github.com/polkadot-js/extension/blob/master/packages/extension-dapp/src/compat/singleSource.ts - in that case it took that object and translated it into the known (supports multiple extensions) interface.

Happy with a compat layer like done above, however direct support for the Metamask layer is going to create too many edge-cases.

jacogr avatar Nov 13 '20 15:11 jacogr

Sounds good, I will take at the compatibility layer. Thanks!

joelamouche avatar Nov 13 '20 16:11 joelamouche

Shout if you need help. While simple it is not exceptionally well documented. But I believe it is do-able - the only issues may be on the signing layer, but there are examples scattered around.

jacogr avatar Nov 13 '20 16:11 jacogr

@jacogr I'm back on this issue.

So if I understand correctly, I need to copy https://github.com/polkadot-js/extension/blob/master/packages/extension-dapp/src/compat/singleSource.ts to adapt injected metamask web3 to conform to the right interface. Is there any way I can test the single source extension injection and extension-dapp when I dev on it?

joelamouche avatar Nov 24 '20 15:11 joelamouche

I will get back to you on that - need to have a refresher first.

jacogr avatar Nov 24 '20 15:11 jacogr

Hi @jacogr , did you get the occasion to look at the code? Thanks in advance

joelamouche avatar Dec 07 '20 11:12 joelamouche

So the best way to test, against apps, is via the following setup -

  • <ROOT>/apps
  • <ROOT>/extension

<ROOT> being any place, just as long as they two repos are on the same level

Then

  • make the changes in the extension, run yarn polkadot-dev-copy-to apps to build and copy the updated packages
  • re-start apps with yarn start to pickup the new changes

jacogr avatar Dec 07 '20 11:12 jacogr

Hi @jacogr

So I pushed some changes on https://github.com/polkadot-js/extension/pull/566

First off, I want to say that the singleSource has probably not been tested because the initCompat function was never called.

Secondly, when I build the apps after running yarn polkadot-dev-copy-to apps I get this error: Screenshot 2020-12-08 at 14 40 09

Please let me know how to add web3 as dependency for this package.

Thanks in advance,

Antoine

joelamouche avatar Dec 08 '20 13:12 joelamouche

It was actually tested and working :) It was however not maintained since the SingleSource app effectively became unused. (I didn't remove the code completely, only since it may have had some use in the future - like now)

It is just a normal monorepo, nothing special. Remove the peerDep, just keep it as a normal dependency.

jacogr avatar Dec 08 '20 13:12 jacogr

Thanks for the help.

So I was able to display the address on polkadot-js/extension#566 but only with a small modififcation on the ui-keyring (https://github.com/polkadot-js/ui/pull/413)

I was not able to get the signer to work. This is not suprising since the signer is never loaded into the app. I did notice that there was a signer option in the signAndSend functions of the signer component.

How should we load the signer? As part of the address object (with updated type) or as a different parallel loading process?

joelamouche avatar Dec 08 '20 17:12 joelamouche

Yes. It gets injected alongside on the same object.

Weird that the original SingleSource doesn’t have the signer anymore - it must be ancient then (aka pre the current signing interfaces)

jacogr avatar Dec 08 '20 17:12 jacogr

Summary of the current state of MetaMask integration into apps:

Now

  • accounts are injected successfully and display with correct balance
  • when implementing signRaw with the eth_sign call, we do trigger the signature on the extension, but nothing gets return
  • MetaMask is planing on discontinuing support for eth_sign in favor of signTypedData and personal_sign because it is a security issue
  • MetaMask appends the signed data with "Ethereum Signature :" message to prevent the signature from being used elsewhere (which is exactly what we are trying to do)

Short term

Since my goal is to support the basic functionality of being able to transfer tokens from the app, we can use the eth_signTransaction call to MM and then post the tx's byte code as an extrinsic (unsigned) to the apps using the .transac call. This would however require to change the signer logic as it is used in the app.

Longer term

We don't know yet if we want to do this, but in the future we will have some governance/staking related features that will need to be called a signed extrinsic and we will need MetaMask to support signing bytecode without anything appended. We don't know if that's something that they want to do but it could be worth asking them.

What do you think @jacogr

joelamouche avatar Dec 11 '20 17:12 joelamouche

Does personal_sign have the same wrapper around it? And signTypedData? If they sign raw bytes, it is possibly worth-while exploring either already.

jacogr avatar Dec 11 '20 17:12 jacogr

https://docs.metamask.io/guide/signing-data.html#a-brief-history https://github.com/ethereum/go-ethereum/pull/2940

personal_sign does prepend data with "\x19Ethereum Signed Message:\n" + len(message). and signTypedData is even worse because it requires the data to be in a json-like format

joelamouche avatar Dec 14 '20 10:12 joelamouche

I was able to make a test proving that MetaMask still supports eth_sign without the prefix : https://github.com/joelamouche/test-dapp/tree/testEthSign

So I'm resuming the effort to integrate MetaMask

https://github.com/polkadot-js/common/pull/969 (ready) https://github.com/polkadot-js/extension/pull/566 (wip) https://github.com/polkadot-js/ui/pull/464 (ready) https://github.com/polkadot-js/apps/pull/5216 (wip)

Are all part of a first milestone: displaying injected accounts from metamask into the app

joelamouche avatar May 07 '21 12:05 joelamouche

@jacogr Great news! Metamask integration was tested with a simple transfer! All PRs are ready but maybe apps should be merged last (see description)

joelamouche avatar May 11 '21 10:05 joelamouche

@albertov19 ready to be tested again

joelamouche avatar Jul 08 '21 14:07 joelamouche