Skip to content
This repository was archived by the owner on Oct 7, 2024. It is now read-only.

Conversation

@soralit
Copy link
Contributor

@soralit soralit commented Sep 26, 2021

Fix the issue that currently MetaMask Extension did not update KeyringController state after forget device so that the addresses should be forgot still remained in redux store and displayed.

Copy link
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.

The KeyringController doesn't currently handle any hardware-wallet-specific methods like this one. In the past we've always called these directly from the extension.

I'm guessing that the problem is that _updateMemStoreKeyrings doesn't get called after forgetDevice is called in the extension. And removeAccount can't be used instead either, because the hardware keyrings don't both implement that method for some reason. So there's no public method to call that would "refresh" the current account state.

I don't love the idea of adding keyring-specific methods 🤔 But the only simple alternative I can think of is a "refreshAccounts" method, and I don't love that idea either because it relies upon the caller to understand the internals of this controller and when accounts need to be refreshed.

So I guess we can accept this for now. And in the new keyring API we'll be proposing soon, we can try to solve this problem in a better way.

Copy link
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!

@Gudahtt Gudahtt merged commit 4b68f5d into MetaMask:main Sep 28, 2021
@soralit soralit deleted the forget-keyring branch October 29, 2021 03:12
@adonesky1 adonesky1 mentioned this pull request Apr 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants