Skip to content
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

Issues with importing ethereum compatible accounts from parity signer app #5589

Closed
joelamouche opened this issue Jun 14, 2021 · 10 comments
Closed

Comments

@joelamouche
Copy link
Contributor

joelamouche commented Jun 14, 2021

Error 1

  1. I use the metadata of moonriver (or other moonbeam related ethereum-compatible network) to create an account on the parity signer app
  2. I try to import it into the account in the browser app
  3. in the console: invalid publicly provided, because the mobile app generates a substrate address instead of an ethereum compatible address
    image (4)

Proposed solution

Popup an error message for ethreum compatible parachains, since users are not supposed to use the mobile app that way

Error 2


  1. Create an ethereum account on mobile app (in the etheruem section of the app)
  2. try to import form ethereum wallet in parity signer into the browser app: Expect substrate/secret address and get ethereum
    image (5)

Proposed Solution

allow ethereum prefix for ethereum compatible parachains

Im off course volunteering to implement the solutions

@jacogr
Copy link
Member

jacogr commented Jun 16, 2021

For Error 1 - we could actually accept both I believe, at least on the keyring side. Since the decoding of the ss58 should just yield the Ethereum address. However, this may be completely broken now that I think of it if we pass the full publicKey there.

On 2, yes, indeed, we should just extend that check.

@joelamouche
Copy link
Contributor Author

So here is the problem with importing ethereum addresses from the mobile signer (related to polkadot-js/extension#758) :
The QR code only provides the address (20bytes) but not the publicKey (32bytes).
In addExternal (ui-keyring):

const pair = this.keyring.addFromAddress(address, _objectSpread(_objectSpread({}, meta), {}, {
      isExternal: true
    }), null);

works well but returns a 20bytes publicKey, which throws an error in saveAccount

What should we do about that?

  • Make saveAccount more tolerant about 20bytes pubKey
  • modify addFromAddress (keyring) to append publicKey with 12 zeros?

@joelamouche
Copy link
Contributor Author

@jacogr please take a look when you have time

@jacogr
Copy link
Member

jacogr commented Jun 23, 2021

I would go for the first option to make it more tolerant - basically if we go for the second, we end up with a publicKey that is completely different and then we will derive a completely different address from it as well.

To go for option 1, not sure what it entails atm, I fear we may need to add something "hacky" somewhere to cater for this.

@joelamouche
Copy link
Contributor Author

this little modification is what it takes : polkadot-js/common#1032 @jacogr

@joelamouche
Copy link
Contributor Author

this PR is necessary polkadot-js/ui#491 also

@joelamouche
Copy link
Contributor Author

@albertov19 please test again and confirm desired behavior

@jacogr
Copy link
Member

jacogr commented Oct 23, 2021

Is this still an ongoing issue?

@joelamouche
Copy link
Contributor Author

Currently the problem is with the Parity Signer mobile app itself and not the app (see novasamatech/parity-signer#787 (comment))

@polkadot-js-bot
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Nov 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants