-
Notifications
You must be signed in to change notification settings - Fork 581
M2 Delivery for Migration Ease #1272
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
Conversation
|
Thanks for the delivery @ainhoa-a I should be able to get to it by the end of the week, thanks for your patience here. |
|
Hi @ainhoa-a, nice to "meet" you again :) Thanks for your submission - I have started looking into it. My feedback so far doesn't strictly speaking pertain to the criteria/deliverables of M2 itself, but since - in my understanding - the app is deemed "complete" (that is: ready for the user horde) after this milestone (with M3 focused on maintenance over the next year), I'd like to address the following few issues I came across before, in my subjective ordering of descending priority:
|
|
Thanks @Lederstrumpf for the feedback. We're already working on some improvements and we'll share a new version soon. |
|
Hi @Lederstrumpf we’ve published a new release that includes the following improvements:
|
|
Hi @ainhoa-a, Thanks for your updated delivery, and apologies for my delayed response.
Thank you, this works well - has the correct level of granularity.
With parallelized rpc calls, I guess you mean that for every network being checked, you query multiple rpc endpoints in parallel?
Ok, so querying/generating addresses from the Ledger is inherently sequential (i.e. this line), but I think the process can still be made much faster (and therefore avoid the need for the user to manually keep the device unlocked) with the following two changes:
So in aggregate, I estimate you'd end up with a ~65% reduction in required unlock time against the status quo if you both 1. segregate address fetching from rpc queries, and 2. only fetch the public key (per account/address index) for a given |
|
Hi @Lederstrumpf please find here the latest release https://github.com/Zondax/polkadot-web-migration/releases/tag/v1.4.0
|
Co-authored-by: Robert Hambrock <[email protected]>
Lederstrumpf
left a comment
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.
Hi @ainhoa-a, thanks for the various updates to the PR.
Happy to accept this PR now, with the caveat of delivery of decision deposit management, as discussed on matrix.
Great work!
|
🪙 Please fill out the invoice form in order to initiate the payment process. Please make sure that you follow the instructions and requirements as laid out in the form as well as our Terms & Conditions. Thank you! |
Milestone Delivery Checklist
Link to the application pull request: w3f/Grants-Program#2437