Skip to content
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

remove extra zero balance account potentially created from seeking ahead #5459

Merged
merged 2 commits into from
Jan 9, 2023

Conversation

rickycodes
Copy link
Member

@rickycodes rickycodes commented Jan 6, 2023

This is to match functionality with: MetaMask/metamask-extension#12074

Explanation:

  • A previous PR added 'seeking' to make sure that when users import their wallets with their seed, consecutive accounts with non-zero balances are automatically generated. However, the PR missed part of the logic: removing the final, zero balance extra account, if created. More motivation behind this PR described in issue https://github.com/MetaMask/metamask-extension/issue/12073.
Before: After
image image
(account 3 is imported with zero eth) (account 3 is removed immediately after import (matching extension functionality))

@rickycodes rickycodes requested a review from a team as a code owner January 6, 2023 17:17
Copy link
Contributor

@owencraston owencraston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we edit/add tests for this functionality similar to the ones here?

Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to prevent adding the zero balance account in the first place rather than removing after it's added? In any case, since this is just a quick patch and doesn't mess with existing logic, LGTM.

@rickycodes
Copy link
Member Author

rickycodes commented Jan 9, 2023

Is there a way to prevent adding the zero balance account in the first place rather than removing after it's added?

unfortunately no. you have to add the account before you can check its balance

@rickycodes rickycodes added No QA Needed/E2E Only Apply this label when your PR does not need any QA effort. release-5.4.0 PRs for v5.4.0 release release-5.14.0 and removed release-5.4.0 PRs for v5.4.0 release labels Jan 9, 2023
@rickycodes rickycodes merged commit 5516975 into main Jan 9, 2023
@rickycodes rickycodes deleted the prevent-extra-acct-on-import branch January 9, 2023 17:45
@github-actions github-actions bot locked and limited conversation to collaborators Jan 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
No QA Needed/E2E Only Apply this label when your PR does not need any QA effort. release-5.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants