From 8346051385fa24208bc524deeace57d28c74739e Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 27 Jan 2026 15:10:20 -0330 Subject: [PATCH 1/3] chore: Simplify TokenListController initialization There was some complexity in handling the order of operations between controller initialization and the event to persist state changes. To simplify this, the controller has been updated such that it no longer persists state changes until _after_ initialization. This makes the logic easier to follow, and lets us delete an instance variable and a few blocks of code. The controller will be initialized as part of wallet initialization, so it will not be "constructed but uninitialized" for any significant length of time. --- .../src/TokenListController.test.ts | 38 ++++++++++++++++ .../src/TokenListController.ts | 43 ++++--------------- 2 files changed, 46 insertions(+), 35 deletions(-) diff --git a/packages/assets-controllers/src/TokenListController.test.ts b/packages/assets-controllers/src/TokenListController.test.ts index 2ead9fa35c4..1b6d13de80b 100644 --- a/packages/assets-controllers/src/TokenListController.test.ts +++ b/packages/assets-controllers/src/TokenListController.test.ts @@ -1414,6 +1414,7 @@ describe('TokenListController', () => { chainId: ChainId.mainnet, messenger: restrictedMessenger, }); + await controller1.initialize(); nock(tokenService.TOKEN_END_POINT_API) .get(getTokensPath(ChainId.mainnet)) @@ -1451,6 +1452,7 @@ describe('TokenListController', () => { chainId: ChainId.mainnet, messenger: restrictedMessenger, }); + await controller.initialize(); await controller.fetchTokenList(ChainId.mainnet); @@ -1473,6 +1475,42 @@ describe('TokenListController', () => { controller.destroy(); }); + it('should not save to StorageService before initialization', async () => { + const messenger = getMessenger(); + const restrictedMessenger = getRestrictedMessenger(messenger); + + // Create controller and fetch tokens + const controller1 = new TokenListController({ + chainId: ChainId.mainnet, + messenger: restrictedMessenger, + }); + // skip initialization + + nock(tokenService.TOKEN_END_POINT_API) + .get(getTokensPath(ChainId.mainnet)) + .reply(200, sampleMainnetTokenList); + + await controller1.fetchTokenList(ChainId.mainnet); + expect( + controller1.state.tokensChainsCache[ChainId.mainnet], + ).toBeDefined(); + + // Wait for debounced persistence to complete (500ms + buffer) + await new Promise((resolve) => setTimeout(resolve, 600)); + + controller1.destroy(); + + // Verify data is in StorageService (per-chain file) + const chainStorageKey = `tokensChainsCache:${ChainId.mainnet}`; + const { result } = await messenger.call( + 'StorageService:getItem', + 'TokenListController', + chainStorageKey, + ); + + expect(result).toBeUndefined(); + }); + it('should handle errors when loading individual chain cache files', async () => { // Pre-populate storage with two chains const validChainData: DataCache = { diff --git a/packages/assets-controllers/src/TokenListController.ts b/packages/assets-controllers/src/TokenListController.ts index c9056334562..033173aa441 100644 --- a/packages/assets-controllers/src/TokenListController.ts +++ b/packages/assets-controllers/src/TokenListController.ts @@ -129,14 +129,6 @@ export class TokenListController extends StaticIntervalPollingController = new Set(); - /** - * Tracks chains that were just loaded from storage and should skip - * the next persistence cycle. This prevents redundant writes where - * data loaded from storage would be immediately written back. - * Chains are removed from this set after being skipped once. - */ - readonly #chainsLoadedFromStorage: Set = new Set(); - /** * Previous tokensChainsCache for detecting which chains changed. */ @@ -211,13 +203,6 @@ export class TokenListController extends StaticIntervalPollingController this.#onCacheChanged(newCache), - (controllerState) => controllerState.tokensChainsCache, - ); - if (onNetworkStateChange) { // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/no-misused-promises @@ -260,13 +245,7 @@ export class TokenListController extends StaticIntervalPollingController 0) { - // Track which chains we're actually loading from storage - // These will be skipped in the next #onCacheChanged to avoid redundant writes - for (const chainId of Object.keys(loadedCache) as Hex[]) { - if (!this.state.tokensChainsCache[chainId]) { - this.#chainsLoadedFromStorage.add(chainId); - } - } - this.update((state) => { // Only load chains that don't already exist in state // This prevents overwriting fresh API data with stale cached data @@ -408,10 +379,6 @@ export class TokenListController extends StaticIntervalPollingController this.#onCacheChanged(newCache), + (controllerState) => controllerState.tokensChainsCache, + ); } } @@ -548,7 +522,6 @@ export class TokenListController extends StaticIntervalPollingController Date: Tue, 27 Jan 2026 17:45:03 -0330 Subject: [PATCH 2/3] Ensure chainlist state updated before init is persisted --- .../src/TokenListController.test.ts | 60 +++++++++++++++++++ .../src/TokenListController.ts | 13 ++-- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/packages/assets-controllers/src/TokenListController.test.ts b/packages/assets-controllers/src/TokenListController.test.ts index 1b6d13de80b..3f557dfe36c 100644 --- a/packages/assets-controllers/src/TokenListController.test.ts +++ b/packages/assets-controllers/src/TokenListController.test.ts @@ -1511,6 +1511,66 @@ describe('TokenListController', () => { expect(result).toBeUndefined(); }); + it('should save data updated before initialization to StorageService', async () => { + // Setup stale mainnet data in storage + const validChainData: DataCache = { + data: sampleMainnetTokensChainsCache, + timestamp: 1, + }; + mockStorage.set( + `TokenListController:tokensChainsCache:${ChainId.mainnet}`, + validChainData, + ); + const messenger = getMessenger(); + const restrictedMessenger = getRestrictedMessenger(messenger); + + // Create controller with delayed initialization, and fetch tokens + const controller = new TokenListController({ + chainId: ChainId.mainnet, + messenger: restrictedMessenger, + }); + + nock(tokenService.TOKEN_END_POINT_API) + .get(getTokensPath(ChainId.mainnet)) + .reply(200, sampleMainnetTokenList); + nock(tokenService.TOKEN_END_POINT_API) + .get(getTokensPath(toHex(56))) + .reply(200, sampleBinanceTokenList); + + await controller.fetchTokenList(ChainId.mainnet); + await controller.fetchTokenList(toHex(56)); + const savedCache = controller.state.tokensChainsCache; + expect(savedCache[ChainId.mainnet]).toBeDefined(); + expect(savedCache[toHex(56)]).toBeDefined(); + await controller.initialize(); + + // Wait for debounced persistence to complete (500ms + buffer) + await new Promise((resolve) => setTimeout(resolve, 600)); + + controller.destroy(); + + // Verify data is in StorageService (per-chain file) + const mainnetStorageKey = `tokensChainsCache:${ChainId.mainnet}`; + const { result: mainnetResult } = await messenger.call( + 'StorageService:getItem', + 'TokenListController', + mainnetStorageKey, + ); + const binanceStorageKey = `tokensChainsCache:${toHex(56)}`; + const { result: binanceResult } = await messenger.call( + 'StorageService:getItem', + 'TokenListController', + binanceStorageKey, + ); + + // Confirm fresh results overwrite stale + expect(mainnetResult).toBeDefined(); + expect(mainnetResult).toStrictEqual(savedCache[ChainId.mainnet]); + // Confirm results not in storage previously are persisted + expect(binanceResult).toBeDefined(); + expect(binanceResult).toStrictEqual(savedCache[toHex(56)]); + }); + it('should handle errors when loading individual chain cache files', async () => { // Pre-populate storage with two chains const validChainData: DataCache = { diff --git a/packages/assets-controllers/src/TokenListController.ts b/packages/assets-controllers/src/TokenListController.ts index 033173aa441..18d3a2c8d01 100644 --- a/packages/assets-controllers/src/TokenListController.ts +++ b/packages/assets-controllers/src/TokenListController.ts @@ -367,6 +367,11 @@ export class TokenListController extends StaticIntervalPollingController 0) { @@ -385,14 +390,8 @@ export class TokenListController extends StaticIntervalPollingController Date: Tue, 27 Jan 2026 17:47:37 -0330 Subject: [PATCH 3/3] Move where subscriber is registered to better match semantic purpose of function --- .../assets-controllers/src/TokenListController.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/assets-controllers/src/TokenListController.ts b/packages/assets-controllers/src/TokenListController.ts index 18d3a2c8d01..78431a25a81 100644 --- a/packages/assets-controllers/src/TokenListController.ts +++ b/packages/assets-controllers/src/TokenListController.ts @@ -229,6 +229,13 @@ export class TokenListController extends StaticIntervalPollingController { await this.#synchronizeCacheWithStorage(); + + // Subscribe to state changes to automatically persist tokensChainsCache + this.messenger.subscribe( + 'TokenListController:stateChange', + (newCache: TokensChainsCache) => this.#onCacheChanged(newCache), + (controllerState) => controllerState.tokensChainsCache, + ); } /** @@ -405,13 +412,6 @@ export class TokenListController extends StaticIntervalPollingController this.#onCacheChanged(newCache), - (controllerState) => controllerState.tokensChainsCache, - ); } }