-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Integrate MetaMask/web3 into polkadot-js interface #4024
Comments
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. |
Sounds good, I will take at the compatibility layer. Thanks! |
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 I'm back on this issue. So if I understand correctly, I need to copy |
I will get back to you on that - need to have a refresher first. |
Hi @jacogr , did you get the occasion to look at the code? Thanks in advance |
So the best way to test, against apps, is via the following setup -
Then
|
Hi @jacogr So I pushed some changes on polkadot-js/extension#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 Please let me know how to add web3 as dependency for this package. Thanks in advance, Antoine |
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. |
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 (polkadot-js/ui#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? |
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) |
Summary of the current state of MetaMask integration into apps:Now
Short termSince 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 termWe 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 |
Does |
https://docs.metamask.io/guide/signing-data.html#a-brief-history 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 |
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 polkadot-js/common#969 (ready) Are all part of a first milestone: displaying injected accounts from metamask into the app |
@jacogr Great news! Metamask integration was tested with a simple transfer! |
@albertov19 ready to be tested again |
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 ?
The text was updated successfully, but these errors were encountered: