Skip to content

Commit 17c67a7

Browse files
salimtbsalimtb
authored
feat: add account api support (#6369)
## Explanation This PR implements functionality for AccountTrackerController to fetch native balances using the AccountsAPI when external services are enabled, with comprehensive test coverage to ensure proper behavior across different configurations. <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## 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 communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent 097610b commit 17c67a7

File tree

5 files changed

+573
-139
lines changed

5 files changed

+573
-139
lines changed

‎eslint-warning-thresholds.json‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@
1414
"n/prefer-global/text-decoder": 1,
1515
"no-shadow": 2
1616
},
17-
"packages/assets-controllers/src/AccountTrackerController.test.ts": {
18-
"import-x/namespace": 2
19-
},
2017
"packages/assets-controllers/src/CurrencyRateController.test.ts": {
2118
"import-x/order": 1,
2219
"jest/no-conditional-in-test": 1

‎packages/assets-controllers/CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- Bump `@metamask/base-controller` from `^8.1.0` to `^8.2.0` ([#6355](https://github.com/MetaMask/core/pull/6355))
1313
- Uses the correct internal account type for the asset ([#6358](https://github.com/MetaMask/core/pull/6358)).
14+
- Enable `AccountTrackerController` to fetch native balances using AccountsAPI when `allowExternalServices` is enabled ([#6369](https://github.com/MetaMask/core/pull/6369))
15+
- Implement native balance fetching via AccountsAPI when `useAccountsAPI` and `allowExternalServices` are both true
16+
- Add fallback to RPC balance fetching when external services are disabled
17+
- Add comprehensive test coverage for both AccountsAPI and RPC balance fetching scenarios
1418

1519
### Fixed
1620

‎packages/assets-controllers/src/AccountTrackerController.test.ts‎

Lines changed: 231 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
getDefaultNetworkControllerState,
88
} from '@metamask/network-controller';
99
import { getDefaultPreferencesState } from '@metamask/preferences-controller';
10-
import * as sinon from 'sinon';
10+
import { useFakeTimers, type SinonFakeTimers } from 'sinon';
1111

1212
import type {
1313
AccountTrackerControllerMessenger,
@@ -58,15 +58,15 @@ const mockedQuery = query as jest.Mock<
5858
>;
5959

6060
describe('AccountTrackerController', () => {
61-
let clock: sinon.SinonFakeTimers;
61+
let clock: SinonFakeTimers;
6262

6363
beforeEach(() => {
64-
clock = sinon.useFakeTimers();
64+
clock = useFakeTimers();
6565
mockedQuery.mockReturnValue(Promise.resolve('0x0'));
6666
});
6767

6868
afterEach(() => {
69-
sinon.restore();
69+
clock.restore();
7070
mockedQuery.mockRestore();
7171
});
7272

@@ -332,7 +332,7 @@ describe('AccountTrackerController', () => {
332332
listAccounts: [ACCOUNT_1, ACCOUNT_2],
333333
networkClientById: {
334334
[networkClientId]: buildCustomNetworkClientConfiguration({
335-
chainId: '0x5',
335+
chainId: '0xe705',
336336
}),
337337
},
338338
},
@@ -348,7 +348,7 @@ describe('AccountTrackerController', () => {
348348
[CHECKSUM_ADDRESS_1]: { balance: '0xa' },
349349
[CHECKSUM_ADDRESS_2]: { balance: '0x0' },
350350
},
351-
'0x5': {
351+
'0xe705': {
352352
[CHECKSUM_ADDRESS_1]: { balance: '0x0' },
353353
[CHECKSUM_ADDRESS_2]: { balance: '0x0' },
354354
},
@@ -369,7 +369,7 @@ describe('AccountTrackerController', () => {
369369
listAccounts: [ACCOUNT_1],
370370
networkClientById: {
371371
[networkClientId]: buildCustomNetworkClientConfiguration({
372-
chainId: '0x5',
372+
chainId: '0xe705',
373373
}),
374374
},
375375
},
@@ -383,7 +383,7 @@ describe('AccountTrackerController', () => {
383383
balance: '0x0',
384384
},
385385
},
386-
'0x5': {
386+
'0xe705': {
387387
[CHECKSUM_ADDRESS_1]: {
388388
balance: '0x10',
389389
},
@@ -407,7 +407,7 @@ describe('AccountTrackerController', () => {
407407
listAccounts: [ACCOUNT_1, ACCOUNT_2],
408408
networkClientById: {
409409
[networkClientId]: buildCustomNetworkClientConfiguration({
410-
chainId: '0x5',
410+
chainId: '0xe705',
411411
}),
412412
},
413413
},
@@ -420,7 +420,7 @@ describe('AccountTrackerController', () => {
420420
[CHECKSUM_ADDRESS_1]: { balance: '0x0' },
421421
[CHECKSUM_ADDRESS_2]: { balance: '0x0' },
422422
},
423-
'0x5': {
423+
'0xe705': {
424424
[CHECKSUM_ADDRESS_1]: { balance: '0x10' },
425425
[CHECKSUM_ADDRESS_2]: { balance: '0x0' },
426426
},
@@ -443,7 +443,7 @@ describe('AccountTrackerController', () => {
443443
listAccounts: [ACCOUNT_1, ACCOUNT_2],
444444
networkClientById: {
445445
[networkClientId]: buildCustomNetworkClientConfiguration({
446-
chainId: '0x5',
446+
chainId: '0xe705',
447447
}),
448448
},
449449
},
@@ -456,7 +456,7 @@ describe('AccountTrackerController', () => {
456456
[CHECKSUM_ADDRESS_1]: { balance: '0x0' },
457457
[CHECKSUM_ADDRESS_2]: { balance: '0x0' },
458458
},
459-
'0x5': {
459+
'0xe705': {
460460
[CHECKSUM_ADDRESS_1]: { balance: '0x11' },
461461
[CHECKSUM_ADDRESS_2]: { balance: '0x12' },
462462
},
@@ -616,6 +616,204 @@ describe('AccountTrackerController', () => {
616616
},
617617
);
618618
});
619+
620+
it('should handle unsupported chains gracefully', async () => {
621+
const networkClientId = 'networkClientId1';
622+
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
623+
await withController(
624+
{
625+
options: {
626+
state: {
627+
accountsByChainId: {
628+
'0x1': {
629+
[CHECKSUM_ADDRESS_1]: { balance: '0x1' },
630+
foo: { balance: '0x2' },
631+
},
632+
'0x2': {
633+
[CHECKSUM_ADDRESS_1]: { balance: '0xa' },
634+
foo: { balance: '0xb' },
635+
},
636+
},
637+
},
638+
},
639+
isMultiAccountBalancesEnabled: true,
640+
selectedAccount: ACCOUNT_1,
641+
listAccounts: [ACCOUNT_1, ACCOUNT_2],
642+
networkClientById: {
643+
[networkClientId]: buildCustomNetworkClientConfiguration({
644+
chainId: '0x5', // Goerli - may not be supported by all balance fetchers
645+
}),
646+
},
647+
},
648+
async ({ controller, refresh }) => {
649+
// Should not throw an error, even for unsupported chains
650+
await refresh(clock, ['networkClientId1']);
651+
652+
// State should still be updated with chain entry from syncAccounts
653+
expect(controller.state.accountsByChainId).toHaveProperty('0x5');
654+
expect(controller.state.accountsByChainId['0x5']).toHaveProperty(
655+
CHECKSUM_ADDRESS_1,
656+
);
657+
expect(controller.state.accountsByChainId['0x5']).toHaveProperty(
658+
CHECKSUM_ADDRESS_2,
659+
);
660+
661+
consoleWarnSpy.mockRestore();
662+
},
663+
);
664+
});
665+
666+
it('should handle timeout error correctly', async () => {
667+
const originalSetTimeout = global.setTimeout;
668+
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();
669+
670+
await withController(
671+
{
672+
options: {
673+
state: {
674+
accountsByChainId: {
675+
'0x1': {
676+
[CHECKSUM_ADDRESS_1]: { balance: '0x1' },
677+
},
678+
},
679+
},
680+
useAccountsAPI: false, // Disable API balance fetchers to force RPC usage
681+
},
682+
isMultiAccountBalancesEnabled: true,
683+
selectedAccount: ACCOUNT_1,
684+
listAccounts: [ACCOUNT_1, ACCOUNT_2],
685+
},
686+
async ({ refresh }) => {
687+
// Mock setTimeout to immediately trigger the timeout callback
688+
global.setTimeout = ((callback: () => void, _delay: number) => {
689+
// This is the timeout callback from line 657 - trigger it immediately
690+
originalSetTimeout(callback, 0);
691+
return 123 as unknown as NodeJS.Timeout; // Return a fake timer id
692+
}) as typeof setTimeout;
693+
694+
// Mock the query to hang indefinitely
695+
const hangingPromise = new Promise(() => {
696+
// Intentionally empty - simulates hanging request
697+
});
698+
mockedQuery.mockReturnValue(hangingPromise);
699+
700+
// Start refresh and let the timeout trigger
701+
await refresh(clock, ['mainnet']);
702+
703+
// Verify that the timeout error was logged (confirms line 657 was executed)
704+
expect(consoleWarnSpy).toHaveBeenCalledWith(
705+
expect.stringContaining(
706+
'Balance fetcher failed for chains 0x1: Error: Timeout after 15000ms',
707+
),
708+
);
709+
710+
// Restore original setTimeout
711+
global.setTimeout = originalSetTimeout;
712+
consoleWarnSpy.mockRestore();
713+
},
714+
);
715+
});
716+
717+
it('should use default allowExternalServices when not provided (covers line 390)', async () => {
718+
// Mock fetch to simulate API balance fetcher behavior
719+
const fetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue({
720+
ok: true,
721+
json: async () => ({ accounts: [] }),
722+
} as Response);
723+
724+
await withController(
725+
{
726+
options: {
727+
useAccountsAPI: true,
728+
// allowExternalServices not provided - should default to () => true (line 390)
729+
},
730+
isMultiAccountBalancesEnabled: true,
731+
selectedAccount: ACCOUNT_1,
732+
listAccounts: [ACCOUNT_1, ACCOUNT_2],
733+
},
734+
async ({ refresh }) => {
735+
// Mock RPC query to return balance
736+
mockedQuery.mockResolvedValue('0x0');
737+
738+
// Refresh balances for mainnet (supported by API)
739+
await refresh(clock, ['mainnet']);
740+
741+
// Since allowExternalServices defaults to () => true (line 390), and useAccountsAPI is true,
742+
// the API fetcher should be used, which means fetch should be called
743+
expect(fetchSpy).toHaveBeenCalled();
744+
745+
fetchSpy.mockRestore();
746+
},
747+
);
748+
});
749+
750+
it('should respect allowExternalServices when set to true', async () => {
751+
// Mock fetch to simulate API balance fetcher behavior
752+
const fetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue({
753+
ok: true,
754+
json: async () => ({ accounts: [] }),
755+
} as Response);
756+
757+
await withController(
758+
{
759+
options: {
760+
useAccountsAPI: true,
761+
allowExternalServices: () => true, // Explicitly set to true
762+
},
763+
isMultiAccountBalancesEnabled: true,
764+
selectedAccount: ACCOUNT_1,
765+
listAccounts: [ACCOUNT_1, ACCOUNT_2],
766+
},
767+
async ({ refresh }) => {
768+
// Mock RPC query to return balance
769+
mockedQuery.mockResolvedValue('0x0');
770+
771+
// Refresh balances for mainnet (supported by API)
772+
await refresh(clock, ['mainnet']);
773+
774+
// Since allowExternalServices is true and useAccountsAPI is true,
775+
// the API fetcher should be used, which means fetch should be called
776+
expect(fetchSpy).toHaveBeenCalled();
777+
778+
fetchSpy.mockRestore();
779+
},
780+
);
781+
});
782+
783+
it('should respect allowExternalServices when set to false', async () => {
784+
// Mock fetch to simulate API balance fetcher behavior
785+
const fetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue({
786+
ok: true,
787+
json: async () => ({ accounts: [] }),
788+
} as Response);
789+
790+
await withController(
791+
{
792+
options: {
793+
useAccountsAPI: true,
794+
allowExternalServices: () => false, // Explicitly set to false
795+
},
796+
isMultiAccountBalancesEnabled: true,
797+
selectedAccount: ACCOUNT_1,
798+
listAccounts: [ACCOUNT_1, ACCOUNT_2],
799+
},
800+
async ({ refresh }) => {
801+
// Mock RPC query to return balance
802+
mockedQuery.mockResolvedValue('0x0');
803+
804+
// Refresh balances for mainnet
805+
await refresh(clock, ['mainnet']);
806+
807+
// Since allowExternalServices is false, the API fetcher should NOT be used
808+
// Only RPC calls should be made, so fetch should NOT be called
809+
expect(fetchSpy).not.toHaveBeenCalled();
810+
// RPC fetcher should be used as the only balance fetcher
811+
// (mockedQuery may or may not be called depending on implementation details)
812+
813+
fetchSpy.mockRestore();
814+
},
815+
);
816+
});
619817
});
620818
});
621819

@@ -684,7 +882,7 @@ describe('AccountTrackerController', () => {
684882
async ({ controller }) => {
685883
jest.spyOn(controller, 'refresh').mockResolvedValue();
686884

687-
await controller.startPolling({
885+
controller.startPolling({
688886
networkClientIds: ['networkClientId1'],
689887
});
690888
await advanceTime({ clock, duration: 1 });
@@ -785,7 +983,7 @@ type WithControllerCallback<ReturnValue> = ({
785983
controller: AccountTrackerController;
786984
triggerSelectedAccountChange: (account: InternalAccount) => void;
787985
refresh: (
788-
clock: sinon.SinonFakeTimers,
986+
clock: SinonFakeTimers,
789987
networkClientIds: NetworkClientId[],
790988
) => Promise<void>;
791989
}) => Promise<ReturnValue> | ReturnValue;
@@ -890,6 +1088,23 @@ async function withController<ReturnValue>(
8901088
'0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000acac5457a3517e0000000000000000000000000000000000000000000027548bd9e4026c918d4b',
8911089
},
8921090
},
1091+
// Mock balanceOf call for zero address - returns same balance data for consistency
1092+
{
1093+
request: {
1094+
method: 'eth_call',
1095+
params: [
1096+
{
1097+
to: '0xcA11bde05977b3631167028862bE2a173976CA11',
1098+
data: '0x70a082310000000000000000000000000000000000000000000000000000000000000000',
1099+
},
1100+
'latest',
1101+
],
1102+
},
1103+
response: {
1104+
result:
1105+
'0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000acac5457a3517e0000000000000000000000000000000000000000000027548bd9e4026c918d4b',
1106+
},
1107+
},
8931108
],
8941109
// eslint-disable-next-line @typescript-eslint/no-explicit-any
8951110
}) as any;
@@ -911,6 +1126,7 @@ async function withController<ReturnValue>(
9111126
...getDefaultNetworkControllerState(),
9121127
chainId: initialChainId,
9131128
});
1129+
9141130
messenger.registerActionHandler(
9151131
'NetworkController:getState',
9161132
mockNetworkState,
@@ -939,7 +1155,7 @@ async function withController<ReturnValue>(
9391155
});
9401156

9411157
const refresh = async (
942-
clock: sinon.SinonFakeTimers,
1158+
clock: SinonFakeTimers,
9431159
networkClientIds: NetworkClientId[],
9441160
) => {
9451161
const promise = controller.refresh(networkClientIds);

0 commit comments

Comments
 (0)