-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Improvement/migrate aes crypto lib #1755
Conversation
…ent/migrate-aes-crypto-lib # Conflicts: # package.json # yarn.lock
app/core/Vault.js
Outdated
* | ||
* @param password - Password to recreate and set the vault with | ||
*/ | ||
export const recreateVault = async (password = '', selectedAddress) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this taking imported accounts or it's going to be deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, QA Passed 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still fails on imported accounts as mentioned in #1756
const { KeyringController, PreferencesController } = Engine.context; | ||
const seedPhrase = await getSeedPhrase(password); | ||
|
||
let importedAccounts = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like is the same code right? https://github.com/MetaMask/metamask-mobile/pull/1756/files#diff-05500d00f4a6959745b57a220987a4b6R359
would be nice to have Logger
here to know what's going on and remove duplicated code, but we can do the last after releasing
Imported accounts are not lost on login 👍 I did notice that we show a different error message on android with empty password or wrong password rather than how in iOS we show Invalid password seen here = https://recordit.co/K8r7qnxUl8 Again this is only on android. To reproduce:
|
@ibrahimtaveras00 just fixed the error message on Android 👍 You can take a look again |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QA Passed 👍
* Update aes crypto lib working * Working on iOS * init migration * Recreate vault on login and fingerprint * Fix selectedAddress * Add passwordSet again * Fix recreate vault * Improve getting simple key pair accounts * Update tests * Make sure selectedAddress is available * Fix for multiple imported accounts * remove unecessary async await * Rename and remove unecessary code * Using logger now * Check new error on android
Description
Migration to the current version of aes-crypto lib from the forked one.