diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 49450a4b687..80c69fcb5dc 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -261,6 +261,9 @@ "packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.test.ts": { "@typescript-eslint/explicit-function-return-type": { "count": 1 + }, + "no-restricted-syntax": { + "count": 1 } }, "packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.ts": { @@ -268,7 +271,10 @@ "count": 2 }, "@typescript-eslint/no-misused-promises": { - "count": 2 + "count": 3 + }, + "no-restricted-syntax": { + "count": 1 } }, "packages/assets-controllers/src/DeFiPositionsController/group-defi-positions.ts": { diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 48b126b36fb..f4272e826c3 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -13,6 +13,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/keyring-controller` from `^25.1.1` to `^25.2.0` ([#8363](https://github.com/MetaMask/core/pull/8363)) - Bump `@metamask/messenger` from `^1.0.0` to `^1.1.1` ([#8364](https://github.com/MetaMask/core/pull/8364), [#8373](https://github.com/MetaMask/core/pull/8373)) +### Fixed + +- `DeFiPositionsController` now refreshes DeFi positions on app launch for returning users ([#8433](https://github.com/MetaMask/core/pull/8433)) + - Previously, `selectedAccountGroupChange` did not fire when the persisted selected group was unchanged, leaving positions stale until the next poll interval. + - Now subscribes to `AccountTreeController:stateChange`, which is guaranteed to fire when `AccountTreeController.init()` calls `this.update()`, triggering a position refresh once accounts are available. + ## [103.1.1] ### Changed diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.test.ts index dd6f2d55957..a0b70f792d5 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.test.ts @@ -105,6 +105,7 @@ function setupController({ 'KeyringController:lock', 'TransactionController:transactionConfirmed', 'AccountTreeController:selectedAccountGroupChange', + 'AccountTreeController:stateChange', ], }); @@ -156,11 +157,16 @@ function setupController({ ); }; + const triggerAccountTreeStateChange = (): void => { + messenger.publish('AccountTreeController:stateChange', {} as never, []); + }; + return { controller, triggerLock, triggerTransactionConfirmed, triggerAccountGroupChange, + triggerAccountTreeStateChange, buildPositionsFetcherSpy, updateSpy, mockFetchPositions, @@ -385,6 +391,71 @@ describe('DeFiPositionsController', () => { expect(updateSpy).toHaveBeenCalledTimes(1); }); + it('fetches positions for the selected evm account when the account tree state changes', async () => { + const mockFetchPositions = jest.fn().mockResolvedValue('mock-fetch-data-1'); + const mockGroupDeFiPositions = jest + .fn() + .mockReturnValue('mock-grouped-data-1'); + + const { + controller, + triggerAccountTreeStateChange, + buildPositionsFetcherSpy, + updateSpy, + } = setupController({ + mockFetchPositions, + mockGroupDeFiPositions, + }); + + triggerAccountTreeStateChange(); + await flushPromises(); + + expect(controller.state).toStrictEqual({ + allDeFiPositions: { + [GROUP_ACCOUNTS[0].address]: 'mock-grouped-data-1', + }, + allDeFiPositionsCount: {}, + }); + + expect(buildPositionsFetcherSpy).toHaveBeenCalled(); + + expect(mockFetchPositions).toHaveBeenCalledWith(GROUP_ACCOUNTS[0].address); + expect(mockFetchPositions).toHaveBeenCalledTimes(1); + + expect(mockGroupDeFiPositions).toHaveBeenCalledWith('mock-fetch-data-1'); + expect(mockGroupDeFiPositions).toHaveBeenCalledTimes(1); + + expect(updateSpy).toHaveBeenCalledTimes(1); + }); + + it('does not fetch positions when the account tree state changes and there is no evm account', async () => { + const { + controller, + triggerAccountTreeStateChange, + buildPositionsFetcherSpy, + updateSpy, + mockFetchPositions, + mockGroupDeFiPositions, + } = setupController({ + mockGroupAccounts: GROUP_ACCOUNTS_NO_EVM, + }); + + triggerAccountTreeStateChange(); + await flushPromises(); + + expect(controller.state).toStrictEqual( + getDefaultDefiPositionsControllerState(), + ); + + expect(buildPositionsFetcherSpy).toHaveBeenCalled(); + + expect(mockFetchPositions).not.toHaveBeenCalled(); + + expect(mockGroupDeFiPositions).not.toHaveBeenCalled(); + + expect(updateSpy).not.toHaveBeenCalled(); + }); + it('does not fetch positions when the account group changes and there is no evm account', async () => { const { controller, diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.ts index 33b33ecc7df..a68b038509c 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.ts @@ -1,6 +1,7 @@ import type { AccountTreeControllerGetAccountsFromSelectedAccountGroupAction, AccountTreeControllerSelectedAccountGroupChangeEvent, + AccountTreeControllerStateChangeEvent, } from '@metamask/account-tree-controller'; import type { ControllerGetStateAction, @@ -114,7 +115,8 @@ export type AllowedActions = export type AllowedEvents = | KeyringControllerLockEvent | TransactionControllerTransactionConfirmedEvent - | AccountTreeControllerSelectedAccountGroupChangeEvent; + | AccountTreeControllerSelectedAccountGroupChangeEvent + | AccountTreeControllerStateChangeEvent; /** * The messenger of the {@link DeFiPositionsController}. @@ -203,6 +205,20 @@ export class DeFiPositionsController extends StaticIntervalPollingController()< }, ); + // `selectedAccountGroupChange` does not fire on returning users when the + // persisted selected group is unchanged. `:stateChange` is guaranteed to + // fire when `AccountTreeController.init()` calls `this.update()`, so we + // use it to catch the initial tree build and trigger a position refresh. + this.messenger.subscribe('AccountTreeController:stateChange', async () => { + const selectedAddress = this.#getSelectedEvmAdress(); + + if (!selectedAddress) { + return; + } + + await this.#updateAccountPositions(selectedAddress); + }); + this.#trackEvent = trackEvent; }