Integrate MetaMask/web3 into polkadot-js interface

polkadot-js / apps

Basic Polkadot/Substrate UI for interacting with a Polkadot and Substrate node. This is the main user-facing application, allowing access to all features available on Substrate chains.

https://dotapps.io

Apache License 2.0

1.73k stars 1.51k forks source link

Integrate MetaMask/web3 into polkadot-js interface #4024

Open joelamouche opened 3 years ago

joelamouche commented 3 years ago

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 ?

jacogr commented 3 years ago

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.

joelamouche commented 3 years ago

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

jacogr commented 3 years ago

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.

joelamouche commented 3 years ago

@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?

jacogr commented 3 years ago

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

joelamouche commented 3 years ago

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

jacogr commented 3 years ago

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

joelamouche commented 3 years ago

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:

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

Thanks in advance,

Antoine

jacogr commented 3 years ago

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.

joelamouche commented 3 years ago

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?

jacogr commented 3 years ago

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)

joelamouche commented 3 years ago

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

jacogr commented 3 years ago

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

joelamouche commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 2 years ago

@albertov19 ready to be tested again