From 4aa3bf4a5fe4e696c738893a6a2bfbf14b2b91b1 Mon Sep 17 00:00:00 2001 From: salimtb Date: Mon, 2 Dec 2024 19:42:27 +0100 Subject: [PATCH 1/4] fix: avoid changing tokens state on ignoreTokens function if we're not on current selected network --- packages/assets-controllers/src/TokensController.test.ts | 4 ++-- packages/assets-controllers/src/TokensController.ts | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/assets-controllers/src/TokensController.test.ts b/packages/assets-controllers/src/TokensController.test.ts index 1f3aa57ea19..418dcec948e 100644 --- a/packages/assets-controllers/src/TokensController.test.ts +++ b/packages/assets-controllers/src/TokensController.test.ts @@ -829,7 +829,7 @@ describe('TokensController', () => { decimals: 6, }); controller.ignoreTokens(['0x03'], InfuraNetworkType.goerli); - expect(controller.state.ignoredTokens).toStrictEqual(['0x03']); + expect(controller.state.ignoredTokens).toStrictEqual([]); // Validate the overall ignored tokens state expect(controller.state.allIgnoredTokens).toStrictEqual({ @@ -879,7 +879,7 @@ describe('TokensController', () => { // Ignore the token on Sepolia controller.ignoreTokens(['0x01'], InfuraNetworkType.sepolia); expect(controller.state.tokens).toHaveLength(0); - expect(controller.state.ignoredTokens).toStrictEqual(['0x01']); + expect(controller.state.ignoredTokens).toStrictEqual([]); // Attempt to ignore a token that was added on Goerli await controller.addToken({ diff --git a/packages/assets-controllers/src/TokensController.ts b/packages/assets-controllers/src/TokensController.ts index feb5293d8c1..05903f48b27 100644 --- a/packages/assets-controllers/src/TokensController.ts +++ b/packages/assets-controllers/src/TokensController.ts @@ -576,7 +576,7 @@ export class TokensController extends BaseController< tokenAddressesToIgnore: string[], networkClientId?: NetworkClientId, ) { - let interactingChainId; + let interactingChainId = this.#chainId; if (networkClientId) { interactingChainId = this.messagingSystem.call( 'NetworkController:getNetworkClientById', @@ -624,12 +624,14 @@ export class TokensController extends BaseController< }); this.update((state) => { - state.ignoredTokens = newIgnoredTokens; - state.tokens = newTokens; state.detectedTokens = newDetectedTokens; state.allIgnoredTokens = newAllIgnoredTokens; state.allDetectedTokens = newAllDetectedTokens; state.allTokens = newAllTokens; + if (interactingChainId === this.#chainId) { + state.tokens = newTokens; + state.ignoredTokens = newIgnoredTokens; + } }); } From 3681c950c8cca5ee8196b828ae5e2a4d87b32319 Mon Sep 17 00:00:00 2001 From: salimtb Date: Tue, 3 Dec 2024 15:23:54 +0100 Subject: [PATCH 2/4] fix: add unit test --- .../src/TokensController.test.ts | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/packages/assets-controllers/src/TokensController.test.ts b/packages/assets-controllers/src/TokensController.test.ts index 418dcec948e..e98e6df441b 100644 --- a/packages/assets-controllers/src/TokensController.test.ts +++ b/packages/assets-controllers/src/TokensController.test.ts @@ -845,6 +845,57 @@ describe('TokensController', () => { ); }); + it('should add tokens to allIgnoredTokens state only if we are not using current network', async () => { + const selectedAddress = '0x0001'; + const selectedAccount = createMockInternalAccount({ + address: selectedAddress, + }); + + await withController( + { + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, + }, + }, + async ({ controller, triggerSelectedAccountChange, changeNetwork }) => { + // Select the first account + triggerSelectedAccountChange(selectedAccount); + + // Add tokens to sepolia + changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); + await controller.addToken({ + address: '0x01', + symbol: 'Token1', + decimals: 18, + }); + expect(controller.state.tokens).toHaveLength(1); + expect(controller.state.ignoredTokens).toHaveLength(0); + + // switch to goerli + changeNetwork({ selectedNetworkClientId: InfuraNetworkType.goerli }); + + // Add tokens to goerli + await controller.addToken({ + address: '0x02', + symbol: 'Token2', + decimals: 8, + }); + + expect(controller.state.tokens).toHaveLength(1); + expect(controller.state.ignoredTokens).toHaveLength(0); + + // ignore token on sepolia + controller.ignoreTokens(['0x01'], InfuraNetworkType.sepolia); + + // as we are not on sepolia, tokens, ignoredTokens, and detectedTokens should not be affected + expect(controller.state.tokens).toHaveLength(1); + expect(controller.state.ignoredTokens).toHaveLength(0); + expect(controller.state.detectedTokens).toHaveLength(0); + }, + ); + }); + it('should not retain ignored tokens from a different network', async () => { const selectedAddress = '0x0001'; const selectedAccount = createMockInternalAccount({ From aa7ded72cfe1fd4cda8388ff416119019f5f85eb Mon Sep 17 00:00:00 2001 From: salimtb Date: Wed, 4 Dec 2024 11:39:51 +0100 Subject: [PATCH 3/4] fix: fix PR comment --- packages/assets-controllers/src/TokensController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/TokensController.ts b/packages/assets-controllers/src/TokensController.ts index 05903f48b27..d0b208a380f 100644 --- a/packages/assets-controllers/src/TokensController.ts +++ b/packages/assets-controllers/src/TokensController.ts @@ -624,11 +624,11 @@ export class TokensController extends BaseController< }); this.update((state) => { - state.detectedTokens = newDetectedTokens; state.allIgnoredTokens = newAllIgnoredTokens; state.allDetectedTokens = newAllDetectedTokens; state.allTokens = newAllTokens; if (interactingChainId === this.#chainId) { + state.detectedTokens = newDetectedTokens; state.tokens = newTokens; state.ignoredTokens = newIgnoredTokens; } From 437a006f7e7354df5f68ab40096fd2d27ac1a3f0 Mon Sep 17 00:00:00 2001 From: salimtb Date: Mon, 9 Dec 2024 01:09:08 +0100 Subject: [PATCH 4/4] fix: add unit tests --- .../src/TokensController.test.ts | 51 ++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/TokensController.test.ts b/packages/assets-controllers/src/TokensController.test.ts index e98e6df441b..5ace73f8911 100644 --- a/packages/assets-controllers/src/TokensController.test.ts +++ b/packages/assets-controllers/src/TokensController.test.ts @@ -845,7 +845,7 @@ describe('TokensController', () => { ); }); - it('should add tokens to allIgnoredTokens state only if we are not using current network', async () => { + it('should not update detectedTokens, tokens, and ignoredTokens state given a network that is different from the globally selected network', async () => { const selectedAddress = '0x0001'; const selectedAccount = createMockInternalAccount({ address: selectedAddress, @@ -896,6 +896,55 @@ describe('TokensController', () => { ); }); + it('should update tokens, and ignoredTokens and detectedTokens state for the globally selected network', async () => { + const selectedAddress = '0x0001'; + const selectedAccount = createMockInternalAccount({ + address: selectedAddress, + }); + + await withController( + { + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, + }, + }, + async ({ controller, triggerSelectedAccountChange, changeNetwork }) => { + // Select the first account + triggerSelectedAccountChange(selectedAccount); + + // Set globally selected network to sepolia + changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); + + // Add a token to sepolia + await controller.addToken({ + address: '0x01', + symbol: 'Token1', + decimals: 18, + }); + // Add a detected token to sepolia + await controller.addDetectedTokens([ + { + address: '0x03', + symbol: 'Token3', + decimals: 18, + }, + ]); + + expect(controller.state.tokens).toHaveLength(1); + expect(controller.state.ignoredTokens).toHaveLength(0); + + // Ignore the token on sepolia + controller.ignoreTokens(['0x01'], InfuraNetworkType.sepolia); + + // Ensure the tokens and ignoredTokens are updated for sepolia (globally selected network) + expect(controller.state.tokens).toHaveLength(0); + expect(controller.state.ignoredTokens).toHaveLength(1); + expect(controller.state.detectedTokens).toHaveLength(1); + }, + ); + }); + it('should not retain ignored tokens from a different network', async () => { const selectedAddress = '0x0001'; const selectedAccount = createMockInternalAccount({