External accounts implemented for issue #5640

[r001]

External acccounts allow Metamask to track account whose private keys are stored on a safe device, and tx data and signatures are exchanged by using QR codes. This allows very safe account password storage.

This pull request enables users to create so called external accounts. This means that after applying this pull request in the "import account" menu there will be an "external" tab that enables importing an ethereum address. There is no local private key stored, but upon confirming a transaction a QR code is presented and the transaction can be signed by an external signer. And the signature can be provided in the signature input field. By providing the signature, the transaction will be signed, and published.

The way it works is the following:

1. User creates transaction (or msg signing) and Confirms it.
2. Request goes to the background, and since the from-accunt belongs to eth-external-account-keyring, the keyrings signTransaction() function is called.
3. Sign transaction adds an object to eth-keyring-controller's extToSign array, which is monitored by the UI modal.js base component.
4. Since there is a new extToSign entry ui/app/components/modals/modal.js starts the external-sign-modal.js window, and displays QR code, and requests signature.
5. Once signature received from user, the tx is segned normall, and handled in a standard way by Metamask.

To allow for external signatures, eth-keyring-controller package had to be updated, and eth-external-account-keyring package was created. So in the package.json my updated ones are the sources.
Eth-external-account-keyring is available at:
https://github.com/r001/eth-external-account-keyring

Install instructions for reference implementation of encoding/decoding QR code can be found at
https://github.com/r001/qr-encoding/blob/master/INSTALL.md

Modified signer that can create appropriate signature can be found at :
https://github.com/r001/dapptools/tree/ethsign-signature
To create signature use ethsign tx --signature-only <tx options>...

[r001]

@caffeinum Cold you pls reintroduce your issues here about visibility. I have rebased the previous pr on top of 5.3.0, and github has not recognized the delete, push again cycle of mine.

[caffeinum]

Of course, here are my objections: r001/qr-encoding#1

The main idea: I think it could be wise to use human-readable parameters for an encoded link instead of raw bytes.

[r001]

@danfinlay I had to rebase pr #5903, so I have reopened here. So you can check it whenever you have time.

[r001]

@caffeinum If it turns out we have to go with the readable format as you have suggested. Do you have time to adjust metamask code, and decoder code? BTW I still think it helps not much, since user can not change anything, since we only return signature.

[caffeinum]

@r001 currently, I am not much available, as there are other projects, but I will look into what I can do when I will have some extra time!

[r001]

@tmashuang Thank you for your help! Meanwhile I could track some additional bugs, and I pushed their fixes too. The test-deps fail because I had to use my own updates for keyring-controller and added external-account-keyring package from my own repo. These need to be put into Metamask's own github place first.
With test-e2e-beta-chrome and test-unit the errors seem to be independent from the implemented new feature.

[tmashuang]

@r001 Merge/rebase develop should fix the test-e2e issue, hopefully also the test-unit. After playing with the behavior, it seems that I can not access the QR tx details after I click out of the view. The tx is pending /awaiting approval in the transaction list. Would be nice to see the QR details again, if the popup closes while viewing the modal. Maybe adding a button to view the QR again in the transaction-list-item-details Also, is the just a proof of concept? How feasible would it be to make to compatible with other signers, i.e. parity signer?

[r001]

@tmashuang

After playing with the behavior, it seems that I can not access the QR tx details after I click out of the view. The tx is pending /awaiting approval in the transaction list. Would be nice to see the QR details again, if the popup closes while viewing the modal.

I'll give it a try. I was testing this with the whole page version of Metamask, not the popup one.

Maybe adding a button to view the QR again in the transaction-list-item-details

If you don't mind, I would only do that after an initial version is merged into develop.

Also, is the just a proof of concept?

No, I want this to be added to Metamask. If there are any quality issues, I'm ready to fix them.

How feasible would it be to make to compatible with other signers, i.e. parity signer?

Absolutely possible. Currently there is a custom made encoding implemented instead of RLP that Parity uses. We can do that too. The hard part of integrating the external feature is already done. Making it compatible with Parity signer is just about how we present the same data.

All the tests check out now except for the test-deps, and that is because of the custom libs I used for updated keyring-controller and external-account-keyring package.

[tmashuang]

I would prefer the have those official external package version bumped, instead of specific commits.
I see one of the KeyringController, will take a look and request another person to review it as well, MetaMask/KeyringController#29.

Could you PR ethereumjs-wallet with you changes as well?

[r001]

Could you PR ethereumjs-wallet with you changes as well?

Makes no sense to PR it since it was an old hackish way of mine trying to introduce external accounts. The original wallet will work just as fine with this version. I have updated package.js accordingly.

EDIT: Now there is a test-unit problem that is unrelated to ethereumjs-wallet. I think I need to do a new rebase, which I can do after we have come back home from holiday on this Friday.

[r001]

This PR has been reopened in PR #6143