fix: throw explicit error when KeyringController is locked

[mikesposito]

Blocked by:

• chore: fix lint warnings in KeyringController #5170

Explanation

No specific error is thrown when an operation that requires an unlocked vault is attempted while KeyringController.state.isUnlocked is false. This doesn't make the operations possible, but it doesn't give a clear error message either.

This PR adds an assertion on almost all KeyringController methods to check if the controller is unlocked and to throw an error when it isn't.

References

• Fixes [KeyringController] No specific error is thrown when the controller is locked #5171

Changelog

@metamask/keyring-controller

• CHANGED: A specific error message is thrown when any operation is attempted while the controller is locked

Checklist

• I've updated the test suite for new or updated code as appropriate
• I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
• I've highlighted breaking changes using the "BREAKING" category above as appropriate
• I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

[mcmire]

@mikesposito What are your thoughts on adding tests for all the cases which we need to assert that the controller is locked? I know there are a lot of methods you're changing, and also there are more changes planned, so not sure if this is unblocking some more work later or if much of this code will disappear later anyway or what.

[mcmire]

Regardless of the above comment, I reviewed this and it looks like the controller does not need to be in unlocked state to call these methods:

• createNewVaultAndRestore
• createNewVaultAndKeychain
• verifyPassword
• isUnlocked
• exportSeedPhrase
• exportAccount
• submitEncryptionKey
• submitPassword

Is that right?

[mikesposito]

@mcmire the following methods should be callable also while the controller is locked:

• createNewVaultAndRestore: the consumer can create a new vault if password is lost
• createNewVaultAndKeychain: when the first vault will be created the wallet will be locked to begin with
• verifyPassword: used to verify the correctness of a password
• isUnlocked: used to check if it is unlocked, and it shouldn't throw errors
• submitEncryptionKey: used to unlock the controller with the encryption key, so it's expected that the wallet is locked while the method is called
• submitPassword: same as above

This one instead has a missing error in the method itself, but getKeyringForAccount is called internally which will throw the error:

• exportAccount

And I forgot about this one:

• exportSeedPhrase: added in f811be0

[mikesposito]

@mikesposito What are your thoughts on adding tests for all the cases which we need to assert that the controller is locked? I know there are a lot of methods you're changing, and also there are more changes planned, so not sure if this is unblocking some more work later or if much of this code will disappear later anyway or what.

This is not being done to unblock later work, and I expect these assertions to stay in place. I think it's good to add explicit tests

[montelaidev]

Lgtm