feat: Accounts controller integration

[owencraston]

Description

1. What is the reason for the change?

• In order to make way for account snaps/multi chain in the future, Metamask is migrating from address based accounts to account managed/created by the accounts controller. This account will be references through an account id and will hold information like the selected address, keyring type and account names (there is other information as well). This change will pave the way for holding snap accounts (which are available on extension already)

3. What is the improvement/solution?

• This change installs and sets up the appropriate actions/event listeners inside the Engine.ts file
• This change also runs a migration which derives the correct account controller information from the preferences controller. This migration runs on the first load to properly setup the data required in the accounts controller. See 032.ts for more information.
• I created a helper method on the Engine class called setSelectedAddress that updates both the AccountsController and PreferencesController. this method ensures that the selected address is always up to date and in sync. There should be NO MORE direct calls to PreferencesController.setSelectedAddress() in the U.I.
• This change DOES NOT interfere with the U.I. Changing the U.I to reference the accounts controller will take place in a subsequent change.

Related issues

Fixes: https://github.com/MetaMask/accounts-planning/issues/305
Equivalent extension PR: MetaMask/metamask-extension#21437

Manual testing steps

Setup

1. checkout branch: accounts-controller
2. run yarn setup && yarn watch:clean
3. in a new terminal run the app via yarn start:ios or inside xcode/android studio
4. optional: download flipper debugger to make inspecting the logs easier

5. add the following logs to app/components/Nav/App/index.js

  // TODO remove this useEffect before merging
  useEffect(() => {
    // eslint-disable-next-line no-console
    console.log(
      'Accounts/ AccountController',
      JSON.stringify(Engine.context.AccountsController.state, null, 2),
    );
    // eslint-disable-next-line no-console
    console.log(
      'Accounts/ PreferencesController',
      JSON.stringify(Engine.context.PreferencesController, null, 2),
    );
  }, []);

Testing

1. create a wallet
2. In your logs (terminal or flipper) search for accounts

• 3. Notice that there are two logs, one for the accounts controller state and one for the preferences controller... your accounts controller log should have the default state

Accounts/ AccountController {
  "internalAccounts": {
    "accounts": {},
    "selectedAccount": ""
  }
}

and the preferences controller should have no selectedAddress or identities

Accounts/ PreferencesController {
  "defaultConfig": {},
  "defaultState": {
      "selectedAddress": "",
     "identities": {},
...

4. create a new account (you should have 2n accounts now)
5. swipe away the app to clear the state and reopen the app (this will trigger the logs again)
6. inspect the logs

• 8. you should have two accounts inside the AccountsControlller state that match the identities inside the preferences controller

Accounts/ AccountController {
  "internalAccounts": {
    "accounts": {
      "7acd47d4-9dd0-4740-842a-31d0ec10f63f": {
        "id": "7acd47d4-9dd0-4740-842a-31d0ec10f63f",
        "address": "0xbedd94abb29f6147481297f05152b18ca8b2633d",
        "options": {},
        "methods": [
          "personal_sign",
          "eth_sign",
          "eth_signTransaction",
          "eth_signTypedData_v1",
          "eth_signTypedData_v3",
          "eth_signTypedData_v4"
        ],
        "type": "eip155:eoa",
        "metadata": {
          "name": "Account 1",
          "keyring": {
            "type": "HD Key Tree"
          },
          "lastSelected": 1709793104305
        }
      },
      "4ca3eac2-9f08-43cd-a39d-3befee58f83b": {
        "id": "4ca3eac2-9f08-43cd-a39d-3befee58f83b",
        "address": "0xb8217d8111997fdaa0578da8f1d13692b90fc246",
        "options": {},
        "methods": [
          "personal_sign",
          "eth_sign",
          "eth_signTransaction",
          "eth_signTypedData_v1",
          "eth_signTypedData_v3",
          "eth_signTypedData_v4"
        ],
        "type": "eip155:eoa",
        "metadata": {
          "name": "Account 2",
          "keyring": {
            "type": "HD Key Tree"
          },
          "lastSelected": 1709793198158
        }
      }
    },
    "selectedAccount": "4ca3eac2-9f08-43cd-a39d-3befee58f83b"
  }
}

Accounts/ PreferencesController {
...
"internalState": {
    "featureFlags": {},
    "identities": {
      "0xbEDd94abB29F6147481297F05152B18Ca8b2633D": {
        "address": "0xbEDd94abB29F6147481297F05152B18Ca8b2633D",
        "name": "Account 1",
        "importTime": 1709793104318
      },
      "0xB8217d8111997fDAA0578Da8f1D13692B90Fc246": {
        "address": "0xB8217d8111997fDAA0578Da8f1D13692B90Fc246",
        "name": "Account 2",
        "importTime": 1709793198278
      }
    },
    "selectedAddress": "0xB8217d8111997fDAA0578Da8f1D13692B90Fc246",
...

• the selectedAddress inside the PreferencesController should match the address for the selectedAccount inside the AccountsController. Since the accounts controller uses account id's instead of the address as a key, you will need to cross reference the account id its properties such as account name and account address.

Caveats

• Make sure to look at the internalState object within the PreferencesController as there are other weird state values that are out of sync and do not reflect the true state of the application.
• You may notice that the addresses inside the AccountsController are all lower cases where as the addresses inside the PreferencesController are checksummed. This is okay since in in the following change we will simply convert the addresses back to checksum in the U.I.

Screenshots/Recordings

Before

• N/A

After

720.Screen.Recording.2024-03-06.at.8.22.46.PM.mov

QA

I personally QA'd this build and tested the following flows. For these test I used this QA build.

Import wallet on a fresh build

• After importing the wallet I added an account
• changed the account names to custom names
• closed the app and reopened it to ensure that the selected address was correct along with the custom name
• connected account to test dapp
• was able to perform signs on the test dapp (personal sign, sign typed data v1-4)
• disconnected account and reconnected
• all addresses and account names were correct during these operations

Video:

Untitled.mp4

Wallet Update Path (testing migration)

• imported a wallet on an older build (7.18) with funds
• added a new account with custom names
• added a QR hardware wallet to this account
• updated the app to my version using the account controller
• after update all accounts were still present with correct names
• funds were all present with correct balances
• selected different account, reloaded the app and successfully found that the selected address was still correct
• connected all 3 account to test dapp
• was able to perform signs on the test dapp (personal sign, sign typed data v1-4) using a QR hardware wallet
• disconnected all 3 accounts from the test dapp
• during this time all account names and addresses were correct
• changed my account name, reloaded the app and found that the new name was preserved
• changed chain, the accounts remained correct

Video:

1000000327.mp4

Dapp connection and send flow

• using the wallet from the above QA session (wallet containing 2 metamask accounts and 1 qr hardware wallet)
• I connected the QR hardware wallet to the test dapp
• was able to perform signs on the test dapp (personal sign, sign typed data v1-4) using a QR hardware wallet
• disconnected the wallet
• reconnected the QR wallet to the test dapp
• went to perform a send
• the selected address was correctly my non QR account
• went through the send flow with the address and names being correct
• I did not send since I did not have enough funds to cover gas costs

Video:

1000000329.mp4

Import account, remove account, re import account

• Using the wallet from the above steps (imported wallet with QR)
• deleted wallet
• re imported wallet
• account was restored correctly
• I was able to successfully connect and perform signs on two different accounts after reimporting

Video:

1000000330.2.mp4

screen-20240320-191514.mp4

Pre-merge author checklist

• I’ve followed MetaMask Coding Standards.
• I've clearly explained what problem this PR is solving and how it is solved.
• I've linked related issues
• I've included manual testing steps
• I've included screenshots/recordings if applicable
• I’ve included tests if applicable
• I’ve documented my code using JSDoc format if applicable
• I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
• I’ve properly set the pull request status:
  • In case it's not yet "ready for review", I've set it to "draft".
  • In case it's "ready for review", I've changed it from "draft" to "non-draft".

Pre-merge reviewer checklist

• I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
• I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[socket-security]

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring: npm/@metamask/obs-store@9.0.0

View full report↗︎

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

[owencraston]

@SocketSecurity ignore npm/@metamask/obs-store@9.0.0

[socket-security]

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@metamask/accounts-controller@10.0.0	Transitive: environment, filesystem, network, shell	+77	15.4 MB	metamaskbot
npm/@metamask/keyring-api@4.0.0	None	+6	471 kB	metamaskbot
npm/@metamask/snaps-controllers@5.0.1	shell Transitive: environment, filesystem	+22	5.79 MB	metamaskbot
npm/@metamask/snaps-sdk@2.1.0	network	+5	724 kB	metamaskbot
npm/@metamask/snaps-utils@6.1.0	filesystem, network, shell	+17	8.38 MB	metamaskbot
npm/fast-xml-parser@4.3.5	None	+1	185 kB	amitgupta
npm/nan@2.19.0	None	0	429 kB	kkoopa

🚮 Removed packages: npm/@metamask/snaps-controllers@5.0.0, npm/@metamask/snaps-sdk@2.0.0, npm/@metamask/snaps-utils@6.0.0, npm/fast-xml-parser@4.2.4, npm/nan@2.18.0

View full report↗︎

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 56.06061% with 58 lines in your changes are missing coverage. Please review.

Project coverage is 45.56%. Comparing base (f031748) to head (ddb5d3d).
Report is 4 commits behind head on main.

Files	Patch %	Lines
app/core/Engine.ts	28.57%	34 Missing and 1 partial ⚠️
app/store/migrations/037.ts	73.17%	6 Missing and 5 partials ⚠️
...mponents/Views/AccountSelector/AccountSelector.tsx	0.00%	2 Missing ⚠️
...ents/Views/AddAccountActions/AddAccountActions.tsx	0.00%	2 Missing ⚠️
app/components/Views/ChoosePassword/index.js	0.00%	2 Missing ⚠️
app/core/Vault.js	0.00%	2 Missing ⚠️
app/util/address/index.js	0.00%	2 Missing ⚠️
app/components/Views/ConnectQRHardware/index.tsx	0.00%	1 Missing ⚠️
app/util/importAdditionalAccounts.js	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8759      +/-   ##
==========================================
+ Coverage   45.50%   45.56%   +0.05%     
==========================================
  Files        1275     1276       +1     
  Lines       31202    31296      +94     
  Branches     3184     3200      +16     
==========================================
+ Hits        14199    14259      +60     
- Misses      16163    16191      +28     
- Partials      840      846       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 56b6b96
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/8555fc07-aa71-41e9-bb3d-a3d119d83792

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[github-actions]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 2384808
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/0c3e60c7-29a1-4190-8a0a-f1341dc34d33

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[github-actions]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: daadd0c
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/aef537b6-1a7f-4e20-997d-c33c06861b7f

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[github-actions]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 9db203a
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/94407799-7db5-4ae4-844d-31d894beb4ba

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[github-actions]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 032b30c
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/6cf441cb-8f8d-45f8-bec3-91550fdc525a

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[sonarcloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
61.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[Cal-L]

LGTM