-
Notifications
You must be signed in to change notification settings - Fork 92
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
perf: remove argon bench on boot, use sane defaults across the board #654
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Tested locally, LGTM pending security green light on this:
- Existing state - unlock native wallet
existing.wallet.mov
- Cleared state - Import wallet
import.mov
- Cleared state - new wallet creation
new.wallet.mov
bbf361a
to
5b3e66f
Compare
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
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.
Solid big wood
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.
Stamping as previously reviewed/tested, implementation didn't change and this has ops green light 🥇
Overview
Hardcodes sensible defaults for argon encryption parameters, skipping the benchmark to reduce initial app boot time by >4s on both desktop and mobile devices, when connecting to native wallet for the first time.
The default parameters remain the same, with only the
iterations
changing from 23 to 16. Noting that reasonably modern mobile devices achieve ~11 iterations within the 1s target, the default was set to a conservative 16 iterations as a balance between performance and security.Testing
This should be tested locally with shapeshift web via local npm registry (e.g verdaccio)
Issue
Will close #5557 after bumping version in web.
Benchmark parameters
Mobile - Pixel 4a
Note overall duration of the bench was 4.1s
Desktop - Mac M2
Note overall duration of the bench was 4.6s
Performance results (on Desktop - Mac M2)
Before changes
4s benchmark runs on first open of native wallet:
After changes
4s benchmark no longer run:
Note the defaults get stored as intended: