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

[Wallet] Validate mnemonic in the app's language #5198

Merged
merged 9 commits into from
Sep 28, 2020
Merged

Conversation

gnardini
Copy link
Contributor

Description

Validating the mnemonic was defaulting to english, so it was not possible to restore accounts created in Spanish or other languages.

It is possible now to restore in Spanish, but only if the app's language is set to Spanish. I wonder if we could do something smarter and figure out the language of the mnemonic based on the first word/s so we don't depend on the language chosen by the user.

Tested

On the simulator

Related issues

Backwards compatibility

Yes

Copy link
Contributor

@jeanregisser jeanregisser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 Thanks for fixing this critical issue!

Copy link
Contributor

@tarikbellamine tarikbellamine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch! Would love to get this in 1.2.0 if the issue was actually that any non-English mnemonics were resulting in errors.

Can we please add some copy to explain to the user that their app language must match their mnemonic language? That should help mitigate that confusion for now.

Copy link
Contributor

@tarikbellamine tarikbellamine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on both iOS and Android just in case and everything looks good. Great work :)

@tarikbellamine tarikbellamine added the automerge Have PR merge automatically when checks pass label Sep 25, 2020
@mergify mergify bot merged commit 14a6cee into master Sep 28, 2020
@mergify mergify bot deleted the mnemonic-language branch September 28, 2020 21:05
ewilz pushed a commit to ewilz/celo-monorepo that referenced this pull request Sep 29, 2020
### Description

Validating the mnemonic was defaulting to english, so it was not possible to restore accounts created in Spanish or other languages.

It is possible now to restore in Spanish, but only if the app's language is set to Spanish. I wonder if we could do something smarter and figure out the language of the mnemonic based on the first word/s so we don't depend on the language chosen by the user.

### Tested

On the simulator

### Related issues

- Fixes celo-org#5192

### Backwards compatibility

Yes
@Lss-Ankit
Copy link

Lss-Ankit commented Oct 2, 2020

@tarikbellamine I have verified this issue on latest test flight build (25) & Android internal build Android internal build 1.2.0(1004294313) and observe issue is still repro
Device: iPhone 11 (14iOS), Samsung Galaxy A70(9.0)
Attachment: Validatemnemonic.mp

@gnardini
Copy link
Contributor Author

gnardini commented Oct 2, 2020

@Lss-Ankit Thanks for testing this! What you see is expected, this change makes it so you can only use an account key of the same language you chose for the app. So, if the app is in Spanish you need a spanish Account Key. To create one, you need to create an account with the language as spanish and the created account key will be in spanish.

@Lss-Ankit
Copy link

Hi @gnardini thank you so much for giving more information about this task

  • I have again verified this task using latest test flight build (25) & Android internal build Android internal build 1.2.0(1004294313) by following scenarios
  • User is able to signin using English account key when user has set up account in English language after sign up.
  • User is able to signin with Spanish account key when user set up account in Spanish language after sign up
  • User not able to signin with English Account key when user has set up account in Spanish language after sign up.
    (User friendly message is shown :Clave de Cuenta invalida )
  • User not able to signin with Spanish Account key when user has set up account in English language after sign up.
    (User friendly message is shown :Invalid account key )
  • User is able to sign up without invite code when user set app language to Spanish
  • User is able to sign up without invite code when user set app language to English

Can you please let me know if i need to test anything else Thanks!

@tarikbellamine
Copy link
Contributor

Sounds like you've covered everything @Lss-Ankit. I think there is no further action needed on this PR :)

@Lss-Ankit
Copy link

Thanks @tarikbellamine for the confirmation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass wallet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mnemonic invalid error for non-English mnemonics
4 participants