core
core copied to clipboard
MetaMask
[keyring-controller] `addAccountWithoutUpdate` now updates the `PreferencesController` state
The KeyringController method addAccountWithoutUpdate no longer works; it does add the account, but not without updating state to include thew new account.
This was broken in @metamask/[email protected]
Maybe we can remove this method instead of fixing it. Will investigate if it's still needed.
It was added originally for this mobile feature: https://github.com/MetaMask/metamask-mobile/pull/1879 But I'm not sure what negative impact temporarily adding the account has; maybe it's OK to add it then remove it during the import process.
Alternatively, we could update the keyring API to add a method that returns the next account address without adding it. That would solve the problem as well.
Alternatively, we could update the keyring API to add a method that returns the next account address without adding it. That would solve the problem as well.
@Gudahtt This could be similar to what also hardware keyrings currently do with getPreviousPage, getNextPage, getFirstPage: they provide a way to seek for accounts before adding them.
Perhaps we could think of a way to standardise that and add it to the general Keyring API? It might be a way to fix this issue while also making the Hardware keyrings easier to use, and a bit more aligned with how other keyrings work
@Gudahtt has offered to test on mobile to determine if the method is even needed.
@Gudahtt Regardless of the test I think we can close this and simply remove the method: addNewAccountWithoutUpdate never worked as intended since calling persistAllKeyrings also updates the controller state along with the vault
This method was intended just to avoid updating PreferencesController state, not all state. The KeyringController state update was not intended to be prevented. I do think it had worked prior to v7 of the PreferencesController.