diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 81d4df26300..3a21fe27941 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Skip `#updateState` assignments whose metadata, balances, or prices are deep-equal to current state so Immer does not publish no-op `stateChange` events ([#10223](https://github.com/MetaMask/core/pull/10223)) + ## [15.1.0] ### Changed diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index dc31450f492..6f9bbc8d4df 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -2208,6 +2208,91 @@ describe('AssetsController', () => { }); }); + describe('when the response repeats data already in state', () => { + const repeatedMetadata: FungibleAssetMetadata = { + type: 'erc20', + symbol: 'USDC', + name: 'USD Coin', + decimals: 6, + }; + const repeatedPrice = { + assetPriceType: 'fungible', + price: 1, + usdPrice: 1, + lastUpdated: 1_700_000_000_000, + } as const; + const repeatedState: Partial = { + assetsInfo: { [MOCK_ASSET_ID]: repeatedMetadata }, + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [MOCK_ASSET_ID]: { amount: '1000000' }, + [MOCK_NATIVE_ASSET_ID]: { amount: '0' }, + }, + }, + assetsPrice: { [MOCK_ASSET_ID]: repeatedPrice }, + }; + const repeatedResponse: DataResponse = { + assetsInfo: { [MOCK_ASSET_ID]: { ...repeatedMetadata } }, + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [MOCK_ASSET_ID]: { amount: '1000000' }, + [MOCK_NATIVE_ASSET_ID]: { amount: '0' }, + }, + }, + assetsPrice: { [MOCK_ASSET_ID]: { ...repeatedPrice } }, + }; + + it('does not publish stateChange', async () => { + await withController( + { state: repeatedState, isBasicFunctionality: () => false }, + async ({ controller, messenger }) => { + await flushPromises(); + const stateChangeListener = jest.fn(); + messenger.subscribe( + 'AssetsController:stateChange', + stateChangeListener, + ); + + await controller.handleAssetsUpdate(repeatedResponse, 'TestSource'); + + expect(stateChangeListener).not.toHaveBeenCalled(); + }, + ); + }); + + it('still publishes stateChange once a single amount changes', async () => { + await withController( + { state: repeatedState, isBasicFunctionality: () => false }, + async ({ controller, messenger }) => { + await flushPromises(); + const stateChangeListener = jest.fn(); + messenger.subscribe( + 'AssetsController:stateChange', + stateChangeListener, + ); + + await controller.handleAssetsUpdate( + { + ...repeatedResponse, + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [MOCK_ASSET_ID]: { amount: '2000000' }, + [MOCK_NATIVE_ASSET_ID]: { amount: '0' }, + }, + }, + }, + 'TestSource', + ); + + expect(stateChangeListener).toHaveBeenCalledTimes(1); + expect( + controller.state.assetsBalance[MOCK_ACCOUNT_ID]?.[MOCK_ASSET_ID], + ).toStrictEqual({ amount: '2000000' }); + }, + ); + }); + }); + it('reconciles a stale native type stored as erc20 when assetsInfo includes the asset', async () => { // Native (zero-address ERC-20) mis-stored as erc20 by an older version. const imxAssetId = diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 4c978938d61..9de861d64a2 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -2708,12 +2708,6 @@ export class AssetsController extends BaseController< for (const [key, value] of Object.entries( normalizedResponse.assetsInfo, )) { - if ( - !isEqual(previousState.assetsInfo[key as Caip19AssetId], value) - ) { - changedMetadata.push(key); - } - const existing = metadata[key] as FungibleAssetMetadata | undefined; const incoming = value as FungibleAssetMetadata; @@ -2722,17 +2716,28 @@ export class AssetsController extends BaseController< // the API). Preserve richer metadata already in state (e.g. from // pendingMetadata set by addCustomAsset) so that the correct // decimals/symbol/name/image are not overwritten with empty values. - if (existing && !incoming.symbol && !incoming.name) { - metadata[key] = { - ...existing, - ...incoming, - symbol: existing.symbol, - name: existing.name, - decimals: existing.decimals ?? incoming.decimals, - image: existing.image ?? incoming.image, - }; - } else { - metadata[key] = value; + const nextMetadata = + existing && !incoming.symbol && !incoming.name + ? { + ...existing, + ...incoming, + symbol: existing.symbol, + name: existing.name, + decimals: existing.decimals ?? incoming.decimals, + image: existing.image ?? incoming.image, + } + : value; + + // Immer detects changes by reference, so assigning a deep-equal + // object still emits a patch and publishes a no-op stateChange. + if ( + !isEqual( + previousState.assetsInfo[key as Caip19AssetId], + nextMetadata, + ) + ) { + changedMetadata.push(key); + metadata[key] = nextMetadata; } } } @@ -2829,7 +2834,12 @@ export class AssetsController extends BaseController< }); } } - balances[accountId] = effective; + // `mergeAccountBalances` always returns a new object. Comparing + // against the raw (possibly `undefined`) entry keeps a first-time + // account write while skipping an unchanged repeat. + if (!isEqual(previousState.assetsBalance[accountId], effective)) { + balances[accountId] = effective; + } } } @@ -2837,7 +2847,9 @@ export class AssetsController extends BaseController< for (const [key, value] of Object.entries( normalizedResponse.assetsPrice, )) { - prices[key] = value; + if (!isEqual(previousPrices[key as Caip19AssetId], value)) { + prices[key] = value; + } } } });