MetaMask / core

This monorepo is a collection of packages used across multiple MetaMask clients
MIT License
250 stars 172 forks source link

Migrate `accounts-controller` to snaps repo #1699

Open legobeat opened 9 months ago

legobeat commented 9 months ago

@metamask/accounts-controller (#1637) has started pulling in dependencies from https://github.com/MetaMask/snaps, which in their turn go back for packages from this repo.

To avoid dependency cycles and release churn arising from this dynamic, I propose migrating the package to the snaps monorepo - at least as long as the relationships look like they currently do.

Hand-drawn ASCII art is the dependency tree limited to packages in MetaMask core and snaps monorepos:

@metamask/accounts-controller(core) -> @metamask/keyring-api ----------> @metamask/rpc-methods(snaps) ---------> @metamask/key-tree
                                   |                                 |                                       |-> @metamask/permission-controller(core)*
                                   |                                 |                                       |-> @metamask/snaps-ui(snaps)*
                                   |                                 |                                       |-> @metamask/snaps-utils(snaps)*
                                   |                                 |
                                   |                                 |-> @metamask/snaps-controllers(snaps) ---> @metamask/approval-controller(core) ------------> @metamask/base-controller(core)*
                                   |                                 |                                       |-> @metamask/base-controller(core)*
                                   |                                 |                                       |-> @metamask/permission-controller(core)*
                                   |                                 |                                       |-> @metamask/rpc-methods(snaps) -------------------> @metamask/key-tree*
                                   |                                 |                                       |                                                 |-> @metamask/permission-controller(core)*
                                   |                                 |                                       |                                                 |-> @metamask/snaps-ui(snaps)*
                                   |                                 |                                       |                                                 |-> @metamask/snaps-utils(snaps)*
                                   |                                 |                                       |
                                   |                                 |                                       |-> @metamask/snaps-execution-environments(snaps) --> @metamask/rpc-methods(snaps)*
                                   |                                 |                                       |                                                 |-> @metamask/snaps-utils(snaps)*
                                   |                                 |                                       |-> @metamask/snaps-utils(snaps)*
                                   |                                 |-> @metamask/snaps-utils(snaps)*
                                   |
                                   |-> @metamask/eth-snap-keyring -----> @metamask/keyring-api*
                                   |                                 |-> @metamask/snaps-controllers(snaps)*
                                   |
                                   |
                                   |-> @metamask/snaps-utils (snaps) --> @metamask/base-controller(core)
                                                                     |-> @metamask/key-tree
                                                                     |-> @metamask/permission-controller(core) --> @metamask/approval-controller(core) --> @metamask/base-controller(core)
                                                                     |                                         |-> @metamask/base-controller(core)
                                                                     |                                         |-> @metamask/controller-utils(core)
                                                                     |-> @metamask/snaps-ui(snaps)
montelaidev commented 9 months ago

Its the the usage of the types SnapsGlobalObject from @metamask/rpc-methods and SnapController from @metamask/snaps-controllers thats importing all the other dependencies. I would prefer not to migrate this to the snaps repo because it isn't related to snaps, and this AccountsController is to be shared between the clients.

Perhaps, a separate types package would be a better solution?

legobeat commented 9 months ago

Naming and exact scope aside, a third repo (poly- or mono) could also be appropriate

Or maybe something like keyrings, which could potentially include keyring-api? :thinking: