Add getKeyringsByType method

[mikesposito]

Description

This PR adds the getKeyringsByType method to KeyringController.
This method can be used to get keyrings of a given type.

To support types from the extension that are not supported by mobile, this function accepts also a string in addition to KeyringTypes - an alternative to this would be to edit the KeyringTypes enum itself but in this controller we don't explicitly support all hardware keyrings.

Note: getKeyringsByType and getKeyringForAccount have been marked as deprecated as their usage is unsafe at the moment: actions executed directly on keyring instances are not being reflected on EthKeyringController and KeyringController states

Changes

• ADDED: getKeyringsByType method to list keyrings for a given type
• DEPRECATED: getKeyringsByType has been marked as deprecated as it is currently unsafe
• DEPRECATED: getKeyringForAccount has been marked as deprecated as it is currently unsafe

References

• Fixes KeyringController: Add getKeyringsByType method #1255

Checklist

• I've updated the test suite for new or updated code as appropriate
• I've updated documentation for new or updated code as appropriate (note: this will usually be JSDoc)
• I've highlighted breaking changes using the "BREAKING" category above as appropriate

[mikesposito]

@Gudahtt I think that we should somehow discourage the use of this method, along with any direct interaction with keyrings. If for example deserialize is called on a keyring, the update will not be propagated to the inner vault and the outer KeyringController, and we'll end up having a stale state.
This can be avoided by calling this.#keyring.persistAllKeyrings, it would be responsibility of the client but that method is inaccessible from the outside.

Do you think we should expose it with a method on KeyringController? This kind of interaction will be needed mainly by the extension

[Gudahtt]

Great point @mikesposito . Calling deserialize on a keyring would indeed be unsafe at the moment. The accounts team has a plan for solving that problem using events, so that the keyring controller can listen to the keyring for updates and always remain in-sync.

In the short term, perhaps we could just deprecate this method and explain why it's dangerous? Then we can be mindful of these dangers in the few places we need it in the extension until we can get rid of it completely.

I am unsure about exposing persistAllKeyrings. Ideally the client would never have a reason to call that, because they would avoid write operations on keyrings directly. If we find a situation like that we can certainly expose it though.

[mikesposito]

Makes sense, I added a deprecation notice for getKeyringsByType and getKeyringForAccount - added also a note on the PR description for visibility

[Gudahtt]

LGTM!