Redesign Keyring Controller API

[Gudahtt]

The keyring controller API is confusing and difficult to extend.

Today we have two "keyring controllers". The first is the original one, eth-keyring-controller, which is used by both projects. The second is @metamask/keyring-controller, which is used by mobile as a wrapper around eth-keyring-controller.

@metamask/keyring-controller mainly serves to coordinate changes between eth-keyring-controller and other controllers. It also handles some keyring-interaction related tasks that aren't handled by eth-keyring-controller, such as input parsing for imported accounts.

• Side-note: In the extension, the closest equivalent to this wrapper class is a collection of keyring-related methods that are directly in metamask-controller.js. This collection of functions serves the same purpose that @metamask/keyring-controller does for mobile.

My questions are:

• What is the KeyringController (eth-keyring-controller) responsible for exactly? Why are some keyring methods called through the KeyringController, while others are called directly on keyrings?
• What is the wrapper KeyringController class responsible for? Can we avoid the need for a wrapper class altogether?
• How can we extend the capabilities of keyrings and integrate keyrings with additional setup requirements without requiring custom methods in the keyring controller?

The outcome of this task should be a proposal in Notion.

Related issues:

• Refactor identity state management #1107
• Consolidate keyring interactions on extension and mobile #1101

Edit: Work towards this has begun, but there is a bunch of preparation required first. Here's a rough checklist of the work involved:

• Introduce new Keyring interface that matches existing APIs as closely as possible
  • Create the types repo and all necessary basic types
  • Create new Keyring interface (work in progress here: Add Keyring types utils#74)
• browser-passworder
  • Standardize and clean up the repository
  • Migrate to TypeScript
  • Rename the repository and package
• obs-store
  • Standardize and clean up the repository
  • Migrate to TypeScript
  • Rename the repository and package
• eth-sig-util
  • Standardize and clean up the repository
  • Migrate to TypeScript
  • Rename the repository and package
• eth-simple-keyring
  • Standardize and clean up the repository
  • Migrate to TypeScript (Migrate to TypeScript eth-simple-keyring#135)
  • Rename the repository and package
• KeyringController
  • Standardize and clean up the repository
  • Migrate to TypeScript (Migrate to TypeScript KeyringController#195)
  • Rename the repository and package
• eth-hd-keyring
  • Standardize and clean up the repository
  • Migrate to TypeScript (Migrate to TypeScript eth-hd-keyring#84)
  • Rename the package
• eth-trezor-keyring
  • Standardize and clean up the repository (Standardize repository eth-trezor-keyring#157)
  • Migrate to TypeScript (Migrate to TypeScript eth-trezor-keyring#156)
  • Rename the package (Rename package to use @metamask scope eth-trezor-keyring#158)
• eth-ledger-bridge-keyring
  • Standardize and clean up the repository (Standardize repo eth-ledger-bridge-keyring#155)
  • Migrate to TypeScript (Migrate to TypeScript eth-ledger-bridge-keyring#23)
  • Rename the package

[Gudahtt]

I'm not sure where I land yet on all of these questions. I have a draft answer for the first one though:

• The KeyringController (eth-keyring-controller) holds references to all keyrings, and is responsible for locking/unlocking, but don't provide pass-through methods for interactiing with keyrings. The rest of the codebase interacts directly with keyrings when performing keyring-related operations.
• The IdentityController holds the "identities" state currently in the PreferencesController, and handles most identity-related functionality that is currently provided by the KeyringController wrapper. Most of the rest of the codebase doesn't deal directly with keys or keyrings - they deal with this "identity" abstraction.
• The AccountTrackerController remains basically as-is, responding to state updates from the IdentityController.

[Gudahtt]

I've begun work to prepare for the new proposed Keyring APIs. It would have been challenging to move directly from what we have now to the new API because we've fallen behind in maintaining these repos and their dependencies. At the moment they're using JavaScript, they're full of inconsistencies and extra non-standard functions, they have outdated dependencies, they don't follow our standard lint rules, etc. So I'll be making an effort to clean them up and migrate them to TypeScript first. This will make more clear what their existing APIs are, so the proposed changes will be easier to understand, review, and implement.

I've added a rough checklist of the work involved to the issue description.

[mcmire]

This may already be covered by the above, but linking this issue in case it gives us ideas: #35

[Gudahtt]

The scope of this issue was too broad. I split out the identity state management to a separate issue, as well as the consolidation of keyring code between extension and mobile.

[Gudahtt]

Closing this in favour of the Keyring Controller API proposal from the accounts team: https://www.notion.so/metamask-consensys/KeyringController-API-b28018e6611940d5a57d5c5a0226902c