Skip to content

Support multiple accounts#48

Merged
Gudahtt merged 5 commits intoMetaMask:masterfrom
rmarkdev:multiple-accounts
Mar 4, 2021
Merged

Support multiple accounts#48
Gudahtt merged 5 commits intoMetaMask:masterfrom
rmarkdev:multiple-accounts

Conversation

@rmarkdev
Copy link
Copy Markdown
Contributor

@rmarkdev rmarkdev commented Dec 8, 2020

This allows to use multiple accounts from one or more ledger devices simultaneously without setting them up repeatedly (MetaMask/metamask-extension#6649).

@rmarkdev rmarkdev requested a review from a team as a code owner December 8, 2020 21:14
@rmarkdev rmarkdev marked this pull request as draft December 16, 2020 09:26
@rmarkdev
Copy link
Copy Markdown
Contributor Author

I noticed this doesn't work properly when using both "legacy" and "ledger live" accounts together. Storing the hdPath for each account might help.

@akaSurreal
Copy link
Copy Markdown

Very happy you are working on this!

@johansten
Copy link
Copy Markdown

Can't wait!

@Arachnid
Copy link
Copy Markdown

We've crowdsourced a substantial bounty if you're able to get this done to the satisfaction of Metamask devs: https://twitter.com/nicksdjohnson/status/1356006896533008384

Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! We're currently testing these changes in this PR: MetaMask/metamask-extension#10505. We should have more feedback relatively soon.

Gudahtt
Gudahtt previously approved these changes Mar 3, 2021
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM, great work! I appreciate that you added so many tests.

Co-authored-by: Mark Stacey <markjstacey@gmail.com>
@darkwing darkwing self-requested a review March 4, 2021 15:37
Copy link
Copy Markdown
Contributor

@darkwing darkwing left a comment

Choose a reason for hiding this comment

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

Excellent, thank you so much @rmarkdev !

@Gudahtt Gudahtt merged commit 639de0f into MetaMask:master Mar 4, 2021
@troublesprouter
Copy link
Copy Markdown

Is this ready for widespread usage? How do I activate this feature? Thank you.

@l3wi
Copy link
Copy Markdown

l3wi commented Apr 16, 2021

@rmarkdev could you please provide an address for the 1.3 ETH that was pledged for when this was merged?

julianariel pushed a commit to block-wallet/eth-ledger-bridge-keyring that referenced this pull request Apr 27, 2022
* allow adding multiple accounts (MetaMask/metamask-extension#6649)

* ensure the unlocked account belongs to the connected device (MetaMask/metamask-extension#6649)

* store hdPath for each account

* refactor unlockAccountByAddress to async function

* improve formatting in _migrateAccountDetails

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants