feat: add and use Accounts API for Account balance calls #4781
Conversation
we currently have added the `getBalances` call, we will extend this in the future
| @@ -0,0 +1,27 @@ | |||
| export type GetBalancesQueryParams = { | |||
There was a problem hiding this comment.
I'm a lazy dev. I tried generating the types via the OpenAPI spec (we do this for notifications)
npx openapi-typescript https://accounts.api.cx.metamask.io/docs-json -o ./src/multi-chain-accounts-service/schema.d.ts -tBut unfortunately the OpenAPI spec is not fully complete or is loosely typed.
If we own this API, it would be cool to tighten the JSDoc, or API spec to improve type generation.
| object: string; | ||
| address: string; | ||
| symbol: string; | ||
| name: string; | ||
| type?: string; | ||
| timestamp?: string; | ||
| decimals: number; | ||
| chainId: number; | ||
| balance: string; |
There was a problem hiding this comment.
This was retrieved from testing the API.
I'll add some JSDoc or expand this once we start using this data.
| */ | ||
| export async function fetchMultiChainBalances( | ||
| address: string, | ||
| options?: GetBalancesQueryParams, |
There was a problem hiding this comment.
Hmm currently a 1:1 mapping for the GET balances query params. We might make this a bit more friendly to work with. Will see from the integration.
There was a problem hiding this comment.
It's important to note that by default the balances API will fetch from multiple networks
https://docs.cx.metamask.io/api/multichainAccounts/#tag/Balances/operation/AccountsController_getAccountBalancesV2
Default: "1,137,56,59144,8453,10,42161,534352"
So will need to see if we should limit this when performing the token detection.
| @@ -0,0 +1,66 @@ | |||
| import type { GetBalancesResponse } from '../types'; | |||
|
|
|||
| export const MOCK_GET_BALANCES_RESPONSE: GetBalancesResponse = { | |||
There was a problem hiding this comment.
Realistic mocks from calling the endpoint.
| // this is where we need to switch + fallback logic. | ||
| // the call is on a slice of 1000 items. Should we bail if fails to get all balances? | ||
| // NOTE - if we hit a rate limit, should we avoid continuing to make the API calls on subsequent slices | ||
| // Lets inspect `getBalancesInSingleCall` to see if this performs any bail or fallback (I think this is a multicall contract call) | ||
| // NOTE - actually we could avoid this loop entirely (since we won't need to recall if retrieved balances from all tokens) | ||
| // NOTE - how should we handle when there are `unprocessedNetworks`? Might be okay since token detection is done at a network level. |
There was a problem hiding this comment.
Hmm I need to think if we need to have this API call done inside this loop (for each token slice), or if it can be done at a top level (per address per chain).
Current Logic:
- Takes ALL tokens in the token list; diffs with token controller; figures out which tokens are missing
- Loops through each missing token to find the balance of that token.
- So there are probably a lot of slices that return
[]with no tokens that have balances.
- So there are probably a lot of slices that return
New Logic:
- Same
- Call Account API - get tokens and their balances - EZ
- Filter out tokens which we have found through the token API
- Continue through Original Step 2 with the pre-filtered list of tokens?
Thoughts on logic
- Only assuming this since the token API could be missing some tokens?
- But not entirely conviced since this new "pre-filtered" list can still be large! Tempted to skip step 4 if we "know" that step 2 could have all the tokens a user has.
getting bearings straight
Prithpal-Sooriya
force-pushed
the
NOTIFY-1179/add-account-api-to-token-detection-controller
branch
from
9bd1664
to
6e4171a
Compare
Explanation
The aim is to attempt to use the Accounts API when making balance calls for expensive functionality (e.g. Token Detection)
This is still under development.
TODO:
References
#4743
https://consensyssoftware.atlassian.net/browse/NOTIFY-1179
Changelog
@metamask/package-a@metamask/package-bChecklist