-
Notifications
You must be signed in to change notification settings - Fork 46
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
Prevent freezing UI rendering while pre-deriving accounts from mnemonic #1454
Conversation
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports |
Deploying with Cloudflare Pages
|
Codecov Report
@@ Coverage Diff @@
## master #1454 +/- ##
==========================================
+ Coverage 83.02% 83.10% +0.07%
==========================================
Files 154 154
Lines 4041 4042 +1
Branches 726 726
==========================================
+ Hits 3355 3359 +4
+ Misses 686 683 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -66,6 +66,7 @@ function* enumerateAccountsFromMnemonic(action: PayloadAction<string>) { | |||
|
|||
try { | |||
yield* setStep(ImportAccountsStep.LoadingAccounts) | |||
// Pre-derive all pages of accounts so we don't need to store mnemonic in redux. | |||
for (let i = 0; i < accountsPerPage * numberOfAccountPages; i++) { | |||
const signer = yield* call(oasis.hdkey.HDKey.getAccountSigner, mnemonic, i) |
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.
does the scheme allow us to do the expensive part once instead of 48 times?
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.
I didn't think of that :D
I currently doesn't https://github.com/oasisprotocol/oasis-sdk/blob/8039a75d7fe42dd8021317424cee073e60435f84/client-sdk/ts-web/core/src/hdkey.ts#L33-L35. it's fine
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.
Fixes #1453