Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate accounts-controller to github.com/MetaMask/accounts #1699

Open
legobeat opened this issue Sep 22, 2023 · 3 comments
Open

Migrate accounts-controller to github.com/MetaMask/accounts #1699

legobeat opened this issue Sep 22, 2023 · 3 comments
Assignees

Comments

@legobeat
Copy link
Contributor

@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)

@legobeat legobeat changed the title Migrate accountrs-controller to snaps repo Migrate accounts-controller to snaps repo Sep 22, 2023
@montelaidev
Copy link
Contributor

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
Copy link
Contributor Author

legobeat commented Sep 25, 2023

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? 🤔

@legobeat
Copy link
Contributor Author

legobeat commented Oct 24, 2024

Now that https://github.com/MetaMask/accounts exists, is that a better candidate than either of https://github.com/MetaMask/snaps or https://github.com/MetaMask/core?

Tentatively changed title accordingly.

@legobeat legobeat changed the title Migrate accounts-controller to snaps repo Migrate accounts-controller to github.com/MetaMask/accounts Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants