MetaMask / MetaMask/core

Merge extension and mobile controllers

Open
#700 5 comments 0 reactions 0 assignees View on GitHub
team-assets team-wallet-framework wf-controller-tech-debt
Dominant language
TypeScript
Stars
413
Forks
308
Avg merge
1d 4h
Merged PRs (30d)
253

Description

We want to use one shared set of controllers on both MetaMask extension and MetaMask mobile. This is to reduce to maintenance burden of each team, so that it is easier for us to provide a stable and secure codebase.

This issue description is still a work in progress. This will be updated with a list of each set of controllers to merge.

Each pair of controllers is listed in this checklist, each with sub-tasks that roughly map out the work required to merge the two controllers.

**Tasks**

#### Extract any API interactions or complex utility functions

Much of the complexity in our controllers is tied to functionality that is better managed as a separate module. Extracting API interactions and complex functionality can simplify the migration process and reduce maintenance.

**Edit:** More context from Mark: This refers to "reducing the scope of what controllers are responsible for (I guess you could call it the single responsibility principle)".

#### Refactor to be more easily understood

Some of our controllers are built in a way that makes them very difficult to reason about. For example, they might expose too many internal methods/properties, or they might have race conditions and complex polling logic. We need to understand how these controllers work in order to merge them, so we may have to spend some time simplifying them before merging in some cases.

**Edit:** More context from Mark: This is a "prompt to think about the controller API and simplifying it ... to improve the code first in cases where it would be really valuable, where it might save time and produce a better result".

#### Write comprehensive unit tests
By writing comprehensive unit tests for the controllers before we merge them, we can reduce the chance of introducing a regression.

#### Normalize the functionality across both controllers

The more similar we make the two controllers prior to merging them, the easier the final step will be.

#### Migrate

Migrate the extension and mobile projects to use the new shared controller. This is the last step.

## Checklist

- [ ] https://github.com/MetaMask/core/issues/1809
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [ ] Write comprehensive unit tests
- [ ] Normalize the functionality across both controllers
- [ ] Remove patches from mobile (pending update to controller packages in mobile)
- [ ] Migrate
- [x] AddressBook
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [x] Write comprehensive unit tests
- [x] Normalize the functionality across both controllers
- [x] Migrate
- [ ] Alert
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [ ] Write comprehensive unit tests
- [x] ~Normalize the functionality across both controllers~ not needed (extension only)
- [ ] Convert to TypeScript
- [ ] Migrate
- [x] Announcement
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [x] Write comprehensive unit tests
- [x] Normalize the functionality across both controllers
- [x] Migrate
- [x] Approval
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [x] Write comprehensive unit tests
- [x] Normalize the functionality across both controllers
- [x] Migrate
- [ ] AppState
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [ ] Write comprehensive unit tests
- [x] ~Normalize the functionality across both controllers~ not needed (only in extension)
- [ ] Convert to TypeScript
- [ ] Migrate
- [ ] AssetsContract
- [ ] Convert to library
- [x] CurrencyRate
- [x] Extract any API interactions or complex utility functions
- [ ] Refactor to be more easily understood: https://github.com/MetaMask/core/issues/1604
- [x] Write comprehensive unit tests
- [x] Normalize the functionality across both controllers
- [x] Migrate
- [x] Ens
- [ ] Extract any API interactions or complex utility functions: https://github.com/MetaMask/core/issues/1605
- [ ] Refactor to be more easily understood: https://github.com/MetaMask/core/issues/1605
- [x] Write comprehensive unit tests
- [x] Normalize the functionality across both controllers
- [x] Migrate https://github.com/MetaMask/metamask-extension/issues/23223
- [x] GasFee
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [ ] Write comprehensive unit tests: https://github.com/MetaMask/core/issues/1600
- [x] Normalize the functionality across both controllers
- [x] Migrate
- [x] Keyring (wrapper) / keyring related metamask-controller methods / Identity related state (#431)
- [ ] Extract any API interactions or complex utility functions
- [ ] Refactor to be more easily understood
- [ ] Write comprehensive unit tests
- [ ] Normalize the functionality across both controllers
- [ ] Migrate
- [x] Message managers
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [x] Write comprehensive unit tests
- [x] Normalize the functionality across both controllers
- [x] Migrate
- [ ] MetaMetrics
- [ ] Extract any API interactions or complex utility functions
- [ ] Refactor to be more easily understood
- [ ] Write comprehensive unit tests
- [ ] ~Normalize the functionality across both controllers~ not needed (extension only)
- [ ] Convert to TypeScript
- [ ] Migrate
- [x] Network (#753)
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [x] Write comprehensive unit tests
- [x] Normalize the functionality across both controllers
- [x] Migrate
- [x] ~Collectibles~ Nft
- [ ] Extract any API interactions or complex utility functions: https://github.com/MetaMask/core/issues/1606
- [ ] Refactor to be more easily understood: https://github.com/MetaMask/core/issues/1606
- [ ] Write comprehensive unit tests: https://github.com/MetaMask/core/issues/1597
- [x] Normalize the functionality across both controllers
- [ ] Remove patches from mobile (pending update to controller packages in mobile)
- [x] Migrate
- [x] NftDetection
- [ ] Extract any API interactions or complex utility functions: https://github.com/MetaMask/core/issues/1607
- [ ] Refactor to be more easily understood: https://github.com/MetaMask/core/issues/1607
- [ ] Write comprehensive unit tests: https://github.com/MetaMask/core/issues/1601
- [x] Normalize the functionality across both controllers
- [x] Migrate
- [x] Notification
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [x] Write comprehensive unit tests
- [x] ~Normalize the functionality across both controllers~ not needed (core only)
- [x] Migrate
- [ ] Onboarding
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [ ] Write comprehensive unit tests
- [ ] ~Normalize the functionality across both controllers~ not needed (extension only)
- [ ] Migrate
- [x] Permission
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [x] Write comprehensive unit tests
- [x] Normalize the functionality across both controllers
- [x] Migrate
- [x] https://github.com/MetaMask/core/issues/1825
- [ ] Extract any API interactions or complex utility functions
- [ ] Refactor to be more easily understood
- [ ] Write comprehensive unit tests
- [ ] ~Normalize the functionality across both controllers~ not needed (extension only)
- [x] Migrate https://github.com/MetaMask/metamask-extension/issues/23181
- [x] Convert to Typescript
- [x] Phishing
- [ ] Extract any API interactions or complex utility functions: https://github.com/MetaMask/core/issues/1608
- [x] Refactor to be more easily understood
- [x] Write comprehensive unit tests
- [x] Normalize the functionality across both controllers
- [x] Migrate
- [x] PPOM
- [ ] Extract any API interactions or complex utility functions
- [ ] Refactor to be more easily understood
- [ ] Write comprehensive unit tests
- [ ] Normalize the functionality across both controllers
- [ ] Migrate
- [ ] Preferences
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [ ] Write comprehensive unit tests: https://github.com/MetaMask/core/issues/1609
- [ ] Normalize the functionality across both controllers
- [ ] Migrate
- [x] [SmartTransactions](https://github.com/MetaMask/smart-transactions-controller)
- [ ] Extract any API interactions or complex utility functions: https://github.com/MetaMask/smart-transactions-controller/issues/188
- [ ] Refactor to be more easily understood: https://github.com/MetaMask/smart-transactions-controller/issues/188
- [ ] Write comprehensive unit tests: https://github.com/MetaMask/smart-transactions-controller/issues/189
- [ ] ~Normalize the functionality across both controllers~ not needed (already extracted)
- [ ] Migrate
- [x] [Snap](https://github.com/MetaMask/snaps/tree/main/packages/snaps-controllers) — not sure if we want to do this or not?
- [ ] Extract any API interactions or complex utility functions
- [ ] Refactor to be more easily understood
- [ ] Write comprehensive unit tests
- [ ] Normalize the functionality across both controllers
- [ ] Migrate
- [x] SubjectMetadata
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [x] Write comprehensive unit tests
- [x] Normalize the functionality across both controllers
- [x] Migrate
- [ ] [Swaps](https://github.com/MetaMask/swaps-controller)
- [ ] Extract any API interactions or complex utility functions: https://github.com/MetaMask/swaps-controller/issues/158
- [ ] Refactor to be more easily understood: https://github.com/MetaMask/swaps-controller/issues/158
- [ ] Write comprehensive unit tests: https://github.com/MetaMask/swaps-controller/issues/159
- [ ] ~Normalize the functionality across both controllers~ not needed (already extracted)
- [ ] Migrate
- [x] TokenList
- [x] Extract any API interactions or complex utility functions
- [ ] Refactor to be more easily understood: https://github.com/MetaMask/core/issues/1610
- [ ] Write comprehensive unit tests: https://github.com/MetaMask/core/issues/1611
- [x] Normalize the functionality across both controllers
- [x] Migrate
- [x] TokenRates
- [x] Extract any API interactions or complex utility functions
- [ ] Refactor to be more easily understood: https://github.com/MetaMask/core/issues/1612
- [ ] Write comprehensive unit tests: https://github.com/MetaMask/core/issues/1598
- [x] Normalize the functionality across both controllers
- [ ] Remove patches from mobile (pending update to controller packages in mobile)
- [x] Migrate
- [x] Tokens
- [ ] Extract any API interactions or complex utility functions: https://github.com/MetaMask/core/issues/1613
- [ ] Refactor to be more easily understood: https://github.com/MetaMask/core/issues/1613
- [ ] Write comprehensive unit tests: https://github.com/MetaMask/core/issues/1599
- [x] Normalize the functionality across both controllers
- [ ] Remove patches from mobile (pending update to controller packages in mobile)
- [x] Migrate
- [x] TokenBalances
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood
- [x] Write comprehensive unit tests
- [x] ~Normalize the functionality across both controllers~ not needed (core only)
- [x] Migrate
- [x] https://github.com/MetaMask/core/issues/1808
- [x] https://github.com/MetaMask/core/issues/1812
- [x] Extract any API interactions or complex utility functions
- [x] Refactor to be more easily understood: https://github.com/MetaMask/core/issues/1614
- [ ] Write comprehensive unit tests: https://github.com/MetaMask/core/issues/1615
- [x] Normalize the functionality across both controllers
- [x] Remove patches from mobile (pending update to controller packages in mobile)
- [x] Migrate https://github.com/MetaMask/metamask-extension/pull/22928
- [x] Transaction / IncomingTransactions — **NOTE: This is being addressed by the Confirmation Systems team**
- [ ] Extract any API interactions or complex utility functions
- [ ] Refactor to be more easily understood
- [ ] Write comprehensive unit tests
- [ ] Normalize the functionality across both controllers
- [ ] Migrate

Contributor guide

No contributing guide indexed for this repository

Research direction

This is an umbrella issue rather than a file-level task, and it names no specific source files or tests. Start by reviewing the unchecked controller entries and their linked MetaMask/core issues, then identify the relevant controller package and existing unit tests. Done requires the selected controller's listed extraction, testing, normalization, conversion, and migration tasks to be complete.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
15/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.