From 760153ee6bb9d47738ecde18751752894aa45cb2 Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Mon, 13 Jan 2025 10:59:22 +0100 Subject: [PATCH 1/9] fix: add thresshold for update NFT metadata --- packages/assets-controllers/src/NftController.ts | 6 +++++- packages/controller-utils/src/constants.ts | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index df159b91a9b..987cfef857c 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -25,6 +25,7 @@ import { ApprovalType, NFT_API_BASE_URL, NFT_API_VERSION, + NFT_UPDATE_THRESHOLD, } from '@metamask/controller-utils'; import { type InternalAccount } from '@metamask/keyring-internal-api'; import type { @@ -2053,7 +2054,10 @@ export class NftController extends BaseController< (singleNft) => !singleNft.name && !singleNft.description && !singleNft.image, ); - if (nftsToUpdate.length !== 0) { + if ( + nftsToUpdate.length !== 0 && + nftsToUpdate.length < NFT_UPDATE_THRESHOLD + ) { await this.updateNftMetadata({ nfts: nftsToUpdate, userAddress: account.address, diff --git a/packages/controller-utils/src/constants.ts b/packages/controller-utils/src/constants.ts index 3b9add77567..3fad08de331 100644 --- a/packages/controller-utils/src/constants.ts +++ b/packages/controller-utils/src/constants.ts @@ -8,6 +8,7 @@ import { export const RPC = 'rpc'; export const FALL_BACK_VS_CURRENCY = 'ETH'; export const IPFS_DEFAULT_GATEWAY_URL = 'https://cloudflare-ipfs.com/ipfs/'; +export const NFT_UPDATE_THRESHOLD = 500; // NETWORKS ID // `toHex` not invoked to avoid cyclic dependency From c679aa0bccb10f2bdb6d87f1a279560b072f3014 Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Mon, 13 Jan 2025 14:43:49 +0100 Subject: [PATCH 2/9] fix: fix add check if incoming values are different than current values --- .../assets-controllers/src/NftController.ts | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index 987cfef857c..a850cf2517c 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -422,15 +422,21 @@ export class NftController extends BaseController< 'AccountsController:getSelectedAccount', ); this.#selectedAccountId = selectedAccount.id; - this.#ipfsGateway = ipfsGateway; - this.#openSeaEnabled = openSeaEnabled; - this.#isIpfsGatewayEnabled = isIpfsGatewayEnabled; - - const needsUpdateNftMetadata = - (isIpfsGatewayEnabled && ipfsGateway !== '') || openSeaEnabled; - - if (needsUpdateNftMetadata && selectedAccount) { - await this.#updateNftUpdateForAccount(selectedAccount); + //Get current state values + if ( + this.#ipfsGateway !== ipfsGateway || + this.#openSeaEnabled !== openSeaEnabled || + this.#isIpfsGatewayEnabled !== isIpfsGatewayEnabled + ) { + this.#ipfsGateway = ipfsGateway; + this.#openSeaEnabled = openSeaEnabled; + this.#isIpfsGatewayEnabled = isIpfsGatewayEnabled; + const needsUpdateNftMetadata = + (isIpfsGatewayEnabled && ipfsGateway !== '') || openSeaEnabled; + + if (needsUpdateNftMetadata && selectedAccount) { + await this.#updateNftUpdateForAccount(selectedAccount); + } } } From a8fc1195b45cd712cef9e421c1c9d3982b338ad0 Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Mon, 13 Jan 2025 15:00:47 +0100 Subject: [PATCH 3/9] fix: test --- .../assets-controllers/src/NftController.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index dc9c9a4a8f2..0038d0bafcf 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -4471,6 +4471,21 @@ describe('NftController', () => { }); describe('updateNftMetadata', () => { + it('should not update Nft metadata when preferences change and current and incoming state are the same', async () => { + const { + nftController, + triggerPreferencesStateChange, + triggerSelectedAccountChange, + } = setupController(); + const spy = jest.spyOn(nftController, 'updateNftMetadata'); + triggerSelectedAccountChange(OWNER_ACCOUNT); + // trigger preference change + triggerPreferencesStateChange({ + ...getDefaultPreferencesState(), + }); + + expect(spy).toHaveBeenCalledTimes(0); + }); it('should update Nft metadata successfully', async () => { const tokenURI = 'https://api.pudgypenguins.io/lil/4'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); From aab4f6b8edf4b2ae667048cac34ff598f48a0597 Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Mon, 13 Jan 2025 15:44:25 +0100 Subject: [PATCH 4/9] fix: test --- .../src/NftController.test.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 0038d0bafcf..941207f9e1b 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -4486,6 +4486,33 @@ describe('NftController', () => { expect(spy).toHaveBeenCalledTimes(0); }); + it('should call update Nft metadata when preferences change is triggered and at least ipfsGateway, openSeaEnabled or isIpfsGatewayEnabled change', async () => { + + const { + nftController, + mockGetAccount, + triggerPreferencesStateChange, + triggerSelectedAccountChange, + } = setupController({ + defaultSelectedAccount: OWNER_ACCOUNT, + }); + const spy = jest.spyOn(nftController, 'updateNftMetadata'); + const testNetworkClientId = 'mainnet'; + mockGetAccount.mockReturnValue(OWNER_ACCOUNT); + await nftController.addNft('0xtest', '3', { + nftMetadata: { name: '', description: '', image: '', standard: '' }, + networkClientId: testNetworkClientId, + }); + + triggerSelectedAccountChange(OWNER_ACCOUNT); + // trigger preference change + triggerPreferencesStateChange({ + ...getDefaultPreferencesState(), + ipfsGateway: 'https://toto/ipfs/', + }); + + expect(spy).toHaveBeenCalledTimes(1); + }); it('should update Nft metadata successfully', async () => { const tokenURI = 'https://api.pudgypenguins.io/lil/4'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); From 82f77b023833583f7f2181bf0a9d7c7bbb07c264 Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Wed, 15 Jan 2025 17:05:33 +0100 Subject: [PATCH 5/9] fix: fix test --- packages/controller-utils/src/index.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/controller-utils/src/index.test.ts b/packages/controller-utils/src/index.test.ts index 61ef841826f..2ada33726a7 100644 --- a/packages/controller-utils/src/index.test.ts +++ b/packages/controller-utils/src/index.test.ts @@ -35,6 +35,7 @@ describe('@metamask/controller-utils', () => { "RPC", "FALL_BACK_VS_CURRENCY", "IPFS_DEFAULT_GATEWAY_URL", + "NFT_UPDATE_THRESHOLD", "GANACHE_CHAIN_ID", "MAX_SAFE_CHAIN_ID", "ERC721", From cb9151766090050c984bce778aae457dde8a6b31 Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Wed, 15 Jan 2025 18:20:09 +0100 Subject: [PATCH 6/9] fix: fix lint comment --- packages/assets-controllers/src/NftController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index a850cf2517c..993c1a87b22 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -422,7 +422,7 @@ export class NftController extends BaseController< 'AccountsController:getSelectedAccount', ); this.#selectedAccountId = selectedAccount.id; - //Get current state values + // Get current state values if ( this.#ipfsGateway !== ipfsGateway || this.#openSeaEnabled !== openSeaEnabled || From 19581f7d88e3afc068da8f8471f68a8b598c9e0d Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Wed, 15 Jan 2025 21:38:13 +0100 Subject: [PATCH 7/9] fix: rm line --- packages/assets-controllers/src/NftController.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 941207f9e1b..eeaad18835e 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -4487,7 +4487,6 @@ describe('NftController', () => { expect(spy).toHaveBeenCalledTimes(0); }); it('should call update Nft metadata when preferences change is triggered and at least ipfsGateway, openSeaEnabled or isIpfsGatewayEnabled change', async () => { - const { nftController, mockGetAccount, From ef12e4f6b91afc5a6feaf5c78305d4efcdc674db Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Thu, 16 Jan 2025 13:07:32 +0100 Subject: [PATCH 8/9] Update packages/assets-controllers/src/NftController.test.ts Co-authored-by: Michele Esposito <34438276+mikesposito@users.noreply.github.com> --- packages/assets-controllers/src/NftController.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index eeaad18835e..49e48178c7a 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -4486,6 +4486,7 @@ describe('NftController', () => { expect(spy).toHaveBeenCalledTimes(0); }); + it('should call update Nft metadata when preferences change is triggered and at least ipfsGateway, openSeaEnabled or isIpfsGatewayEnabled change', async () => { const { nftController, @@ -4512,6 +4513,7 @@ describe('NftController', () => { expect(spy).toHaveBeenCalledTimes(1); }); + it('should update Nft metadata successfully', async () => { const tokenURI = 'https://api.pudgypenguins.io/lil/4'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); From 023dd64c9441a88fc798c4d1e4685067821cd041 Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Thu, 16 Jan 2025 13:08:16 +0100 Subject: [PATCH 9/9] fix: remove import --- packages/assets-controllers/src/NftController.ts | 14 +++++++------- packages/controller-utils/src/constants.ts | 1 - packages/controller-utils/src/index.test.ts | 1 - 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index 993c1a87b22..292611dc0d7 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -25,7 +25,6 @@ import { ApprovalType, NFT_API_BASE_URL, NFT_API_VERSION, - NFT_UPDATE_THRESHOLD, } from '@metamask/controller-utils'; import { type InternalAccount } from '@metamask/keyring-internal-api'; import type { @@ -101,12 +100,11 @@ type SuggestedNftMeta = { * @property isCurrentlyOwned - Boolean indicating whether the address/chainId combination where it's currently stored currently owns this NFT * @property transactionId - Transaction Id associated with the NFT */ -export type Nft = - | { - tokenId: string; - address: string; - isCurrentlyOwned?: boolean; - } & NftMetadata; +export type Nft = { + tokenId: string; + address: string; + isCurrentlyOwned?: boolean; +} & NftMetadata; type NftUpdate = { nft: Nft; @@ -275,6 +273,8 @@ export const getDefaultNftControllerState = (): NftControllerState => ({ ignoredNfts: [], }); +const NFT_UPDATE_THRESHOLD = 500; + /** * Controller that stores assets and exposes convenience methods */ diff --git a/packages/controller-utils/src/constants.ts b/packages/controller-utils/src/constants.ts index 3fad08de331..3b9add77567 100644 --- a/packages/controller-utils/src/constants.ts +++ b/packages/controller-utils/src/constants.ts @@ -8,7 +8,6 @@ import { export const RPC = 'rpc'; export const FALL_BACK_VS_CURRENCY = 'ETH'; export const IPFS_DEFAULT_GATEWAY_URL = 'https://cloudflare-ipfs.com/ipfs/'; -export const NFT_UPDATE_THRESHOLD = 500; // NETWORKS ID // `toHex` not invoked to avoid cyclic dependency diff --git a/packages/controller-utils/src/index.test.ts b/packages/controller-utils/src/index.test.ts index 2ada33726a7..61ef841826f 100644 --- a/packages/controller-utils/src/index.test.ts +++ b/packages/controller-utils/src/index.test.ts @@ -35,7 +35,6 @@ describe('@metamask/controller-utils', () => { "RPC", "FALL_BACK_VS_CURRENCY", "IPFS_DEFAULT_GATEWAY_URL", - "NFT_UPDATE_THRESHOLD", "GANACHE_CHAIN_ID", "MAX_SAFE_CHAIN_ID", "ERC721",