-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix: cp-7.42.0 improve performance of notifications loading when opening the app #13803
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
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.
Thanks for all the context you added in your comments! LGTM!
Also, approving since there may be not enough time to get this directly in the controllers for this release, but the contents of this PR will be replaced soon by an implementation directly in UserStorageController
|
|
|
Description
There is serious performance impact of loading notifications on startup. It is roughly a 2 second lag, and the app freezes during this process.
This is due to us processing the encryption key to be used for decrypting entries in UserStorage.
This persists the a cached encryption key using existing storage mechanisms to securely store this hashed key.
NOTE: This fix is an interim solution until we add e2e content keys for encryption (CC @mathieuartu @mirceanis)
MetaMask/core#5129
Related issues
Fixes:
Manual testing steps
EXPECTED - you should not see any lag or app freezing.
Screenshots/Recordings
Before
https://www.loom.com/share/8951a714b9b34f79a3e988ed50f9f656?sid=ad2ff732-c893-49d6-bd4b-e7f13a3a08b5
After
https://www.loom.com/share/fe0afadcd49b46bbbafe219f95777fb6?sid=ca0df6aa-b5c5-4ffd-a5ee-bc811f0f1058
Pre-merge author checklist
Pre-merge reviewer checklist