diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 938840f74e2..9a831f40e24 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Improve controller constructor by moving async logic to it's own method ([#5781](https://github.com/MetaMask/core/pull/5781)) - Updated `TokenListController` `fetchTokenList` method to bail if cache is valid ([#5804](https://github.com/MetaMask/core/pull/5804)) - also cleaned up internal state update logic - Bump `@metamask/controller-utils` to `^11.9.0` ([#5812](https://github.com/MetaMask/core/pull/5812)) diff --git a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts index 7fe8810d168..931d988bbb5 100644 --- a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts +++ b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts @@ -342,6 +342,9 @@ describe('MultichainBalancesController', () => { it('stores balances when receiving new balances from the "AccountsController:accountBalancesUpdated" event', async () => { const { controller, messenger } = setupController(); + + await controller.initialize(); + const balanceUpdate = { balances: { [mockBtcAccount.id]: mockBalanceResult, @@ -403,6 +406,8 @@ describe('MultichainBalancesController', () => { }, }); + await controller.initialize(); + await waitForAllPromises(); expect(controller.state.balances[mockBtcAccount.id]).toStrictEqual( @@ -573,6 +578,8 @@ describe('MultichainBalancesController', () => { }, }); + await controller.initialize(); + mockSnapHandleRequest.mockReset(); mockListMultichainAccounts.mockReset(); @@ -642,4 +649,52 @@ describe('MultichainBalancesController', () => { mockBalanceResult, ); }); + + it('initializes by fetching balances for non-EVM accounts', async () => { + const { controller, mockSnapHandleRequest } = setupController({ + mocks: { + listMultichainAccounts: [mockBtcAccount], + }, + }); + + expect(controller.state.balances).toStrictEqual({}); + + await controller.initialize(); + await waitForAllPromises(); + + expect(mockSnapHandleRequest).toHaveBeenCalled(); + expect(controller.state.balances[mockBtcAccount.id]).toStrictEqual( + mockBalanceResult, + ); + }); + + it('handles errors during initialization', async () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); + + const initError = new Error('Test initialization error'); + + const { controller, mockListMultichainAccounts } = setupController({ + mocks: { + listMultichainAccounts: [mockBtcAccount], + }, + }); + + mockListMultichainAccounts.mockReturnValue([mockBtcAccount]); + + const updateSpy = jest.spyOn(controller, 'updateBalance'); + updateSpy.mockRejectedValue(initError); + + await controller.initialize(); + await waitForAllPromises(); + + expect(consoleErrorSpy).toHaveBeenCalledWith( + `Failed to fetch initial balances for account ${mockBtcAccount.id}:`, + initError, + ); + + expect(controller.state.balances).toStrictEqual({}); + + updateSpy.mockRestore(); + consoleErrorSpy.mockRestore(); + }); }); diff --git a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts index e39372874d1..67169ca6f27 100644 --- a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts +++ b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts @@ -156,13 +156,6 @@ export class MultichainBalancesController extends BaseController< }, }); - // Fetch initial balances for all non-EVM accounts - for (const account of this.#listAccounts()) { - // Fetching the balance is asynchronous and we cannot use `await` here. - // eslint-disable-next-line no-void - void this.updateBalance(account.id); - } - this.messagingSystem.subscribe( 'AccountsController:accountRemoved', (account: string) => this.#handleOnAccountRemoved(account), @@ -187,6 +180,21 @@ export class MultichainBalancesController extends BaseController< ); } + /** + * Initialize the controller by fetching initial balances for all non-EVM accounts. + * This method should be called after the controller is constructed. + */ + async initialize(): Promise { + for (const account of this.#listAccounts()) { + this.updateBalance(account.id).catch((error) => { + console.error( + `Failed to fetch initial balances for account ${account.id}:`, + error, + ); + }); + } + } + /** * Updates the balances for the given accounts. * diff --git a/packages/multichain-transactions-controller/CHANGELOG.md b/packages/multichain-transactions-controller/CHANGELOG.md index 6f50eb0e889..d7189d1f7ad 100644 --- a/packages/multichain-transactions-controller/CHANGELOG.md +++ b/packages/multichain-transactions-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Improve controller constructor by moving async logic to it's own method ([#5781](https://github.com/MetaMask/core/pull/5781)) + ## [1.0.0] ### Changed diff --git a/packages/multichain-transactions-controller/src/MultichainTransactionsController.test.ts b/packages/multichain-transactions-controller/src/MultichainTransactionsController.test.ts index 326f7d6a6a7..a8618c2c10f 100644 --- a/packages/multichain-transactions-controller/src/MultichainTransactionsController.test.ts +++ b/packages/multichain-transactions-controller/src/MultichainTransactionsController.test.ts @@ -337,7 +337,7 @@ describe('MultichainTransactionsController', () => { expect( Object.keys(controller.state.nonEvmTransactions[mockSolAccount.id]), - ).toHaveLength(4); + ).toHaveLength(3); expect( controller.state.nonEvmTransactions[mockSolAccount.id][ @@ -951,4 +951,33 @@ describe('MultichainTransactionsController', () => { transactions[1], ); }); + + it('handles errors when initializing transactions for accounts', async () => { + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + const { controller, mockSnapHandleRequest } = setupController({ + mocks: { + listMultichainAccounts: [mockBtcAccount], + }, + }); + + mockSnapHandleRequest.mockRejectedValueOnce( + new Error('Failed to fetch transactions'), + ); + + jest + .spyOn(controller, 'updateTransactionsForAccount') + .mockRejectedValueOnce(new Error('Failed to fetch transactions')); + + await controller.initialize(); + await waitForAllPromises(); + + expect(consoleSpy).toHaveBeenCalledWith( + `Failed to fetch initial transactions for account ${mockBtcAccount.id}:`, + expect.any(Error), + ); + + expect(controller.state).toStrictEqual({ nonEvmTransactions: {} }); + + consoleSpy.mockRestore(); + }); }); diff --git a/packages/multichain-transactions-controller/src/MultichainTransactionsController.ts b/packages/multichain-transactions-controller/src/MultichainTransactionsController.ts index b36b3c667e3..dc0864e80a8 100644 --- a/packages/multichain-transactions-controller/src/MultichainTransactionsController.ts +++ b/packages/multichain-transactions-controller/src/MultichainTransactionsController.ts @@ -189,16 +189,6 @@ export class MultichainTransactionsController extends BaseController< }, }); - // Fetch initial transactions for all non-EVM accounts - for (const account of this.#listAccounts()) { - this.updateTransactionsForAccount(account.id).catch((error) => { - console.error( - `Failed to fetch initial transactions for account ${account.id}:`, - error, - ); - }); - } - this.messagingSystem.subscribe( 'AccountsController:accountAdded', (account: InternalAccount) => this.#handleOnAccountAdded(account), @@ -214,6 +204,21 @@ export class MultichainTransactionsController extends BaseController< ); } + /** + * Initialize the controller by fetching initial transactions for all non-EVM accounts. + * This method should be called after the controller is constructed. + */ + async initialize(): Promise { + for (const account of this.#listAccounts()) { + this.updateTransactionsForAccount(account.id).catch((error) => { + console.error( + `Failed to fetch initial transactions for account ${account.id}:`, + error, + ); + }); + } + } + /** * Lists the multichain accounts coming from the `AccountsController`. *