feat: add and use Accounts API for Account balance calls #4781

Prithpal-Sooriya · 2024-10-10T14:39:45Z

Explanation

This integrates the Accounts API (Multi-chain Balances Endpoint) to help alleviate expensive RPC calls made by Token Detection.

The aim is to attempt to use the Accounts API when making balance calls for expensive functionality (e.g. Token Detection)

Code Walkthrough

https://www.loom.com/share/e540cae3967746b0aca343d4c59d0af6?sid=69c2556c-96d3-451e-bd67-7d03f32fff03

References

#4743

https://consensyssoftware.atlassian.net/browse/NOTIFY-1179

Changelog

`@metamask/assets-controllers`

ADDED: MultiChain Accounts Service
- ADDED: fetchSupportedNetworks() function to dynamically fetch supported networks by the Accounts API
- ADDED: fetchMultiChainBalances() function to get balances for a given address
ADDED: useAccountsAPI to the TokenDetectionController constructor to enable/disable the accounts API feature.
ADDED: #addDetectedTokensViaAPI() private method in TokenDetectionController to get detected tokens via the Accounts API.
CHANGED: detectTokens() method in TokenDetectionController to try AccountsAPI first before using RPC flow to detect tokens.

Checklist

I've updated the test suite for new or updated code as appropriate
I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
I've highlighted breaking changes using the "BREAKING" category above as appropriate
I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

we currently have added the `getBalances` call, we will extend this in the future

packages/assets-controllers/src/multi-chain-accounts-service/types.ts

packages/assets-controllers/src/multi-chain-accounts-service/multi-chain-accounts.ts

packages/assets-controllers/src/multi-chain-accounts-service/mocks/mock-get-balances.ts

packages/assets-controllers/src/TokenDetectionController.ts

getting bearings straight

…account-api-to-token-detection-controller

and add jsdoc comments to balance response type

…troller. I'll add some tests and might refactor out pieces if it makes it easier for testing

Prithpal-Sooriya · 2024-10-11T13:56:13Z

packages/assets-controllers/src/TokenDetectionController.ts

   */
  constructor({
    interval = DEFAULT_INTERVAL,
    disabled = true,
    getBalancesInSingleCall,
    trackMetaMetricsEvent,
    messenger,
+    useAccountsAPI = true,


Big proponent of feature switching (in case this PR gets too large, or we need to turn this feature off).

Happy to remove otherwise.

This is enabled by default

packages/assets-controllers/src/TokenDetectionController.ts

Prithpal-Sooriya · 2024-10-11T18:26:54Z

packages/assets-controllers/src/TokenDetectionController.test.ts

+    /**
+     * TODO - discuss if this is correct.
+     *
+     * If the Accounts API succeeds, but the tokens cannot be added (unable to create `Token` shape)
+     * Then should we add no tokens & then finish?
+     *
+     * If we want to, we could then do a pass with the RPC flow? But would it be necessary?
+     * - DEV - we can just add a simple check at the end of the API flow where if no tokens were added, then count it as a failure and perform the RPC flow?
+     */


Yeah lets discuss this scenario.

This is because we are using the TokenCache to create the correct token shape.
The Token shape requires the aggregators and image fields which the API does not provide us.

If the new token from the API does not exist in this cache, then we need to skip it.

Prithpal-Sooriya · 2024-10-11T18:27:57Z

packages/assets-controllers/src/TokenDetectionController.test.ts

+    /**
+     * TODO - discuss if this is correct.
+     *
+     * If the Accounts API succeeds, but the tokens cannot be added (token already added)
+     * Then should we add no tokens (if they are all added) & then finish?
+     *
+     * If we want to, we could then do a pass with the RPC flow? But would it be necessary?
+     * - DEV - we can just add a simple check at the end of the API flow where if no tokens were added, then count it as a failure and perform the RPC flow?
+     */


Yep lets discuss.

This flow happens when we receive tokens from our API, but these tokens are already added into the wallet (as detected tokens, ignored tokens, etc).

So if the token is already added, we will skip it.

Prithpal-Sooriya · 2024-10-11T18:54:13Z

packages/assets-controllers/src/TokenDetectionController.ts

+        // NOTE: some of this data is also available from the `token` balance,
+        // however it is missing the `aggregators` and `iconUrl` properties


These 2 fields that are missing from the Account API are technically optional. We could leave them out.

However I do not know the unintended effects if we were to do this.

packages/assets-controllers/src/TokenDetectionController.test.ts

…account-api-to-token-detection-controller

packages/assets-controllers/src/TokenDetectionController.ts

currently the supportedNetworks call has places that has unreachable errors, but could still be errors if used directly in the future

…account-api-to-token-detection-controller

Prithpal-Sooriya · 2024-10-15T12:14:38Z

@metamaskbot publish-preview

github-actions · 2024-10-15T12:18:22Z

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.

{
  "@metamask-previews/accounts-controller": "18.2.2-preview-200beb79",
  "@metamask-previews/address-book-controller": "6.0.1-preview-200beb79",
  "@metamask-previews/announcement-controller": "7.0.1-preview-200beb79",
  "@metamask-previews/approval-controller": "7.1.0-preview-200beb79",
  "@metamask-previews/assets-controllers": "38.3.0-preview-200beb79",
  "@metamask-previews/base-controller": "7.0.1-preview-200beb79",
  "@metamask-previews/build-utils": "3.0.1-preview-200beb79",
  "@metamask-previews/chain-controller": "0.1.3-preview-200beb79",
  "@metamask-previews/composable-controller": "9.0.1-preview-200beb79",
  "@metamask-previews/controller-utils": "11.3.0-preview-200beb79",
  "@metamask-previews/ens-controller": "14.0.1-preview-200beb79",
  "@metamask-previews/eth-json-rpc-provider": "4.1.4-preview-200beb79",
  "@metamask-previews/gas-fee-controller": "20.0.1-preview-200beb79",
  "@metamask-previews/json-rpc-engine": "9.0.3-preview-200beb79",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.3-preview-200beb79",
  "@metamask-previews/keyring-controller": "17.2.2-preview-200beb79",
  "@metamask-previews/logging-controller": "6.0.1-preview-200beb79",
  "@metamask-previews/message-manager": "10.1.1-preview-200beb79",
  "@metamask-previews/name-controller": "8.0.1-preview-200beb79",
  "@metamask-previews/network-controller": "21.0.1-preview-200beb79",
  "@metamask-previews/notification-controller": "7.0.0-preview-200beb79",
  "@metamask-previews/notification-services-controller": "0.9.0-preview-200beb79",
  "@metamask-previews/permission-controller": "11.0.2-preview-200beb79",
  "@metamask-previews/permission-log-controller": "3.0.1-preview-200beb79",
  "@metamask-previews/phishing-controller": "12.0.3-preview-200beb79",
  "@metamask-previews/polling-controller": "10.0.1-preview-200beb79",
  "@metamask-previews/preferences-controller": "13.0.3-preview-200beb79",
  "@metamask-previews/profile-sync-controller": "0.9.7-preview-200beb79",
  "@metamask-previews/queued-request-controller": "5.1.0-preview-200beb79",
  "@metamask-previews/rate-limit-controller": "6.0.1-preview-200beb79",
  "@metamask-previews/selected-network-controller": "18.0.1-preview-200beb79",
  "@metamask-previews/signature-controller": "19.1.0-preview-200beb79",
  "@metamask-previews/transaction-controller": "37.2.0-preview-200beb79",
  "@metamask-previews/user-operation-controller": "15.0.1-preview-200beb79"
}

…on-controller

packages/assets-controllers/src/TokenDetectionController.ts

…account-api-to-token-detection-controller

…er' of github.com:MetaMask/core into NOTIFY-1179/add-account-api-to-token-detection-controller

…on-controller

bergeron · 2024-10-16T16:10:54Z

packages/assets-controllers/src/TokenDetectionController.ts

+          return;
+        }
+
+        // We need specific data from tokenList to correctly create a token


Are the fields we get from the token list the icon url and aggregators?

We'll eventually stop storing icon urls in state since its always calculable as "https://static.cx.metamask.io/api/v1/tokenIcons/${chainId}/${address}.png".

Aggregators are shown in the UI but we could just query them from the big token list rather than duplicating on each imported token. So I think we keep this for now but could remove the dependency later.

Yep 100% agree. How expensive would it be for the accounts API to serve this additional information?

Aha saw a relevant comment you've made before, great callout.
https://github.com/MetaMask/mobile-planning/issues/1935#issuecomment-2384061477

bergeron · 2024-10-16T16:13:30Z

packages/assets-controllers/src/TokenDetectionController.ts

+      }
+
+      const result = await fetchSupportedNetworks().catch(() => null);
+      this.supportedNetworksCache = result;


I like the cache and dynamic lookup of support. Since its in memory rather than state, it'll re run when extension is reloaded or browser close/open. But not each time you open the popup. Which seems like a reasonable interval.

Yep only on browser close/open. We can think of more sophisticating caches if need be... later though 😄

bergeron · 2024-10-16T16:14:23Z

packages/assets-controllers/src/multi-chain-accounts-service/multi-chain-accounts.ts

@@ -0,0 +1,52 @@
+import { handleFetch } from '@metamask/controller-utils';


Nice move with the separate client. Also good to attempt OpenAPI generation but we can live without it for now.

bergeron

This looks great to me, I couldn't find any bugs. I tested chains with and without account API support, hiding tokens, and manually adding them.

…on-controller

feat: add multichain accounts API services

7cee69b

we currently have added the `getBalances` call, we will extend this in the future

Prithpal-Sooriya commented Oct 10, 2024

View reviewed changes

packages/assets-controllers/src/multi-chain-accounts-service/types.ts Show resolved Hide resolved

Prithpal-Sooriya commented Oct 10, 2024

View reviewed changes

packages/assets-controllers/src/multi-chain-accounts-service/types.ts Outdated Show resolved Hide resolved

Prithpal-Sooriya commented Oct 10, 2024

View reviewed changes

packages/assets-controllers/src/multi-chain-accounts-service/multi-chain-accounts.ts Outdated Show resolved Hide resolved

Prithpal-Sooriya commented Oct 10, 2024

View reviewed changes

packages/assets-controllers/src/multi-chain-accounts-service/mocks/mock-get-balances.ts Show resolved Hide resolved

Prithpal-Sooriya commented Oct 10, 2024

View reviewed changes

packages/assets-controllers/src/TokenDetectionController.ts Outdated Show resolved Hide resolved

docs: scaffold and comment sections of TokenDetectionController

6e4171a

getting bearings straight

Prithpal-Sooriya force-pushed the NOTIFY-1179/add-account-api-to-token-detection-controller branch from 9bd1664 to 6e4171a Compare October 10, 2024 16:17

Prithpal-Sooriya added 4 commits October 11, 2024 09:54

Merge branch 'main' of github.com:MetaMask/core into NOTIFY-1179/add-…

b52827e

…account-api-to-token-detection-controller

feat: add accounts API support networks endpoint

d201250

refactor: rename fetchSupportedNetworks function

816ca78

and add jsdoc comments to balance response type

feat: add token detection via API method and connect to detection con…

d4c37ef

…troller. I'll add some tests and might refactor out pieces if it makes it easier for testing

Prithpal-Sooriya commented Oct 11, 2024

View reviewed changes

packages/assets-controllers/src/TokenDetectionController.ts Show resolved Hide resolved

Prithpal-Sooriya added 2 commits October 11, 2024 15:09

refactor: add barrel file

9f5724d

test: add test coverage for tokenDetectionController

9adebed

Prithpal-Sooriya force-pushed the NOTIFY-1179/add-account-api-to-token-detection-controller branch from 12a4687 to 9adebed Compare October 11, 2024 18:23

Prithpal-Sooriya commented Oct 11, 2024

View reviewed changes

packages/assets-controllers/src/TokenDetectionController.test.ts Outdated Show resolved Hide resolved

Prithpal-Sooriya added 2 commits October 14, 2024 13:24

Merge branch 'main' of github.com:MetaMask/core into NOTIFY-1179/add-…

4e08cba

…account-api-to-token-detection-controller

docs: update test util jsdoc comment

0911070

Prithpal-Sooriya commented Oct 14, 2024

View reviewed changes

packages/assets-controllers/src/TokenDetectionController.ts Show resolved Hide resolved

Prithpal-Sooriya added 2 commits October 14, 2024 14:47

docs: remove todo/note comments

8664211

test: add ignore comments for untestable statements

228515c

currently the supportedNetworks call has places that has unreachable errors, but could still be errors if used directly in the future

Prithpal-Sooriya marked this pull request as ready for review October 14, 2024 13:52

Prithpal-Sooriya requested a review from a team as a code owner October 14, 2024 13:52

Prithpal-Sooriya added the team-assets label Oct 14, 2024

Prithpal-Sooriya requested a review from a team October 14, 2024 13:56

Merge branch 'main' of github.com:MetaMask/core into NOTIFY-1179/add-…

200beb7

…account-api-to-token-detection-controller

Merge branch 'main' into NOTIFY-1179/add-account-api-to-token-detecti…

4a464ab

…on-controller

darkwing reviewed Oct 15, 2024

View reviewed changes

packages/assets-controllers/src/TokenDetectionController.ts Outdated Show resolved Hide resolved

Prithpal-Sooriya added 4 commits October 16, 2024 12:58

Merge branch 'main' of github.com:MetaMask/core into NOTIFY-1179/add-…

87c4155

…account-api-to-token-detection-controller

refactor: use existing enums and constants

cc53352

Merge branch 'NOTIFY-1179/add-account-api-to-token-detection-controll…

7157e10

…er' of github.com:MetaMask/core into NOTIFY-1179/add-account-api-to-token-detection-controller

Merge branch 'main' into NOTIFY-1179/add-account-api-to-token-detecti…

9bd0f70

…on-controller

bergeron reviewed Oct 16, 2024

View reviewed changes

bergeron approved these changes Oct 16, 2024

View reviewed changes

darkwing approved these changes Oct 16, 2024

View reviewed changes

Prithpal-Sooriya added 2 commits October 17, 2024 15:03

Merge branch 'main' into NOTIFY-1179/add-account-api-to-token-detecti…

56e242c

…on-controller

Merge branch 'main' into NOTIFY-1179/add-account-api-to-token-detecti…

909fde8

…on-controller

Prithpal-Sooriya merged commit 1251b86 into main Oct 18, 2024
116 checks passed

Prithpal-Sooriya deleted the NOTIFY-1179/add-account-api-to-token-detection-controller branch October 18, 2024 10:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add and use Accounts API for Account balance calls #4781

feat: add and use Accounts API for Account balance calls #4781

Prithpal-Sooriya commented Oct 10, 2024 •

edited

Loading

Prithpal-Sooriya Oct 11, 2024

Prithpal-Sooriya Oct 15, 2024

Prithpal-Sooriya Oct 11, 2024

Prithpal-Sooriya Oct 11, 2024

Prithpal-Sooriya Oct 11, 2024

Prithpal-Sooriya commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

bergeron Oct 16, 2024

Prithpal-Sooriya Oct 17, 2024

Prithpal-Sooriya Oct 17, 2024 •

edited

Loading

bergeron Oct 16, 2024

Prithpal-Sooriya Oct 17, 2024

bergeron Oct 16, 2024

bergeron left a comment

		// NOTE: some of this data is also available from the `token` balance,
		// however it is missing the `aggregators` and `iconUrl` properties

		@@ -0,0 +1,52 @@
		import { handleFetch } from '@metamask/controller-utils';

feat: add and use Accounts API for Account balance calls #4781

feat: add and use Accounts API for Account balance calls #4781

Conversation

Prithpal-Sooriya commented Oct 10, 2024 • edited Loading

Explanation

References

Changelog

@metamask/assets-controllers

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Prithpal-Sooriya commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Prithpal-Sooriya Oct 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bergeron left a comment

Choose a reason for hiding this comment

Prithpal-Sooriya commented Oct 10, 2024 •

edited

Loading

`@metamask/assets-controllers`

Prithpal-Sooriya Oct 17, 2024 •

edited

Loading