-
-
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
refactor: migrate Encryptor to TypeScript and increase PBKDF2 iterations number #9093
Conversation
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring: Next stepsTake a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with |
3b72a9e
to
5516e4a
Compare
Bitrise❌❌❌ Commit hash: 8777bac Note
|
@SocketSecurity ignore npm/@metamask/eth-keyring-controller@15.1.0 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9093 +/- ##
==========================================
+ Coverage 45.55% 45.66% +0.10%
==========================================
Files 1272 1273 +1
Lines 31238 31263 +25
Branches 3189 3198 +9
==========================================
+ Hits 14232 14275 +43
+ Misses 16167 16149 -18
Partials 839 839 ☔ View full report in Codecov by Sentry. |
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.
LGTM! Awesome refactor on the Encryptor class
Bitrise✅✅✅ Commit hash: 8efbf16 Note
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@metamask/keyring-controller@8.1.0 |
Bitrise❌❌❌ Commit hash: b3ff44f Note
|
Bitrise❌❌❌ Commit hash: 429d8cd Note
|
Bitrise❌❌❌ Commit hash: 3a70ab2 Note
|
This reverts commit e3a4e66.
…k/metamask-mobile into refactor/encryptor-class
Bitrise✅✅✅ Commit hash: ccf8052 Note
|
Quality Gate passedIssues Measures |
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.
LGTM. Just left minor remarks that we can handle later on! :)
Description
📣 The code in this PR has already been reviewed and QA'd in #8828
This PR introduces the following modifications
@metamask/keyring-controller
bump fromv8.1.0
tov9.0.0
CHANGELOG from
@metamask/keyring-controller
Related issues
Fixes: https://github.com/MetaMask/accounts-planning/issues/100
Manual testing steps
Test case 1: Upgrade the client from the current version to feature branch
Test case 2: Lock and unlock the wallet
Test case 3: Reveal SRP and private key
Test case 4: Connect QR wallet
Test case 5: Connect Ledger wallet
Test case 6: Sign a transaction with each account type
Test case 7: Sign a different messages with each account type
Test case 8: Change password
Test case 9: Unlock with biometrics
Test case 10: Remember me
Test case 11: Onboarding with biometrics
Unit Tests and Coverage
Screenshots/Recordings
Not applicable. No changes to the UI/UX.
Pre-merge author checklist
Pre-merge reviewer checklist