diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 2c5004e1c3..e2bba3891a 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -1380,7 +1380,7 @@ describe('NftController', () => { nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: ERC721_KUDOSADDRESS, - image: 'Kudos Image (directly from tokenURI)', + image: 'url', name: 'Kudos Name (directly from tokenURI)', description: 'Kudos Description (directly from tokenURI)', tokenId: ERC721_KUDOS_TOKEN_ID, @@ -3917,5 +3917,108 @@ describe('NftController', () => { expect(spy).toHaveBeenCalledTimes(1); }); + + it('should call getNftInformation only one time per interval', async () => { + const tokenURI = 'https://api.pudgypenguins.io/lil/4'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController, triggerPreferencesStateChange } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); + const selectedAddress = OWNER_ADDRESS; + const spy = jest.spyOn(nftController, 'updateNft'); + const testNetworkClientId = 'sepolia'; + await nftController.addNft('0xtest', '3', { + nftMetadata: { name: '', description: '', image: '', standard: '' }, + networkClientId: testNetworkClientId, + }); + + nock('https://api.pudgypenguins.io/lil').get('/4').reply(200, { + name: 'name pudgy', + image: 'url pudgy', + description: 'description pudgy', + }); + const testInputNfts: Nft[] = [ + { + address: '0xtest', + description: null, + favorite: false, + image: null, + isCurrentlyOwned: true, + name: null, + standard: 'ERC721', + tokenId: '3', + tokenURI: 'https://api.pudgypenguins.io/lil/4', + }, + ]; + + // Make first call to updateNftMetadata should trigger state update + await nftController.updateNftMetadata({ + nfts: testInputNfts, + networkClientId: testNetworkClientId, + }); + expect(spy).toHaveBeenCalledTimes(1); + + expect( + nftController.state.allNfts[selectedAddress][SEPOLIA.chainId][0], + ).toStrictEqual({ + address: '0xtest', + description: 'description pudgy', + image: 'url pudgy', + name: 'name pudgy', + tokenId: '3', + standard: 'ERC721', + favorite: false, + isCurrentlyOwned: true, + tokenURI: 'https://api.pudgypenguins.io/lil/4', + }); + + spy.mockClear(); + + // trigger calling updateNFTMetadata again on the same account should not trigger state update + const spy2 = jest.spyOn(nftController, 'updateNft'); + await nftController.updateNftMetadata({ + nfts: testInputNfts, + networkClientId: testNetworkClientId, + }); + // No updates to state should be made + expect(spy2).toHaveBeenCalledTimes(0); + + // trigger preference change and change selectedAccount + const testNewAccountAddress = 'OxDifferentAddress'; + triggerPreferencesStateChange({ + ...getDefaultPreferencesState(), + selectedAddress: testNewAccountAddress, + }); + + spy.mockClear(); + await nftController.addNft('0xtest', '4', { + nftMetadata: { name: '', description: '', image: '', standard: '' }, + networkClientId: testNetworkClientId, + }); + + const testInputNfts2: Nft[] = [ + { + address: '0xtest', + description: null, + favorite: false, + image: null, + isCurrentlyOwned: true, + name: null, + standard: 'ERC721', + tokenId: '4', + tokenURI: 'https://api.pudgypenguins.io/lil/4', + }, + ]; + + const spy3 = jest.spyOn(nftController, 'updateNft'); + await nftController.updateNftMetadata({ + nfts: testInputNfts2, + networkClientId: testNetworkClientId, + }); + // When the account changed, and updateNftMetadata is called state update should be triggered + expect(spy3).toHaveBeenCalledTimes(1); + }); }); }); diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index 83fdeae592..998552b9d3 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -729,7 +729,7 @@ export class NftController extends BaseController< name: blockchainMetadata?.name ?? nftApiMetadata?.name ?? null, description: blockchainMetadata?.description ?? nftApiMetadata?.description ?? null, - image: blockchainMetadata?.image ?? nftApiMetadata?.image ?? null, + image: nftApiMetadata?.image ?? blockchainMetadata?.image ?? null, standard: blockchainMetadata?.standard ?? nftApiMetadata?.standard ?? null, tokenURI: blockchainMetadata?.tokenURI ?? null, @@ -1450,56 +1450,62 @@ export class NftController extends BaseController< userAddress?: string; networkClientId?: NetworkClientId; }) { - const chainId = this.#getCorrectChainId({ networkClientId }); + const releaseLock = await this.#mutex.acquire(); - const nftsWithChecksumAdr = nfts.map((nft) => { - return { - ...nft, - address: toChecksumHexAddress(nft.address), - }; - }); - const nftMetadataResults = await Promise.all( - nftsWithChecksumAdr.map(async (nft) => { - const resMetadata = await this.#getNftInformation( - nft.address, - nft.tokenId, - networkClientId, - ); + try { + const chainId = this.#getCorrectChainId({ networkClientId }); + + const nftsWithChecksumAdr = nfts.map((nft) => { return { - nft, - newMetadata: resMetadata, + ...nft, + address: toChecksumHexAddress(nft.address), }; - }), - ); - - // We want to avoid updating the state if the state and fetched nft info are the same - const nftsWithDifferentMetadata: NftUpdate[] = []; - const { allNfts } = this.state; - const stateNfts = allNfts[userAddress]?.[chainId] || []; - - nftMetadataResults.forEach((singleNft) => { - const existingEntry: Nft | undefined = stateNfts.find( - (nft) => - nft.address.toLowerCase() === singleNft.nft.address.toLowerCase() && - nft.tokenId === singleNft.nft.tokenId, + }); + const nftMetadataResults = await Promise.all( + nftsWithChecksumAdr.map(async (nft) => { + const resMetadata = await this.#getNftInformation( + nft.address, + nft.tokenId, + networkClientId, + ); + return { + nft, + newMetadata: resMetadata, + }; + }), ); - if (existingEntry) { - const differentMetadata = compareNftMetadata( - singleNft.newMetadata, - existingEntry, + // We want to avoid updating the state if the state and fetched nft info are the same + const nftsWithDifferentMetadata: NftUpdate[] = []; + const { allNfts } = this.state; + const stateNfts = allNfts[userAddress]?.[chainId] || []; + + nftMetadataResults.forEach((singleNft) => { + const existingEntry: Nft | undefined = stateNfts.find( + (nft) => + nft.address.toLowerCase() === singleNft.nft.address.toLowerCase() && + nft.tokenId === singleNft.nft.tokenId, ); - if (differentMetadata) { - nftsWithDifferentMetadata.push(singleNft); + if (existingEntry) { + const differentMetadata = compareNftMetadata( + singleNft.newMetadata, + existingEntry, + ); + + if (differentMetadata) { + nftsWithDifferentMetadata.push(singleNft); + } } - } - }); + }); - if (nftsWithDifferentMetadata.length !== 0) { - nftsWithDifferentMetadata.forEach((elm) => - this.updateNft(elm.nft, elm.newMetadata, userAddress, chainId), - ); + if (nftsWithDifferentMetadata.length !== 0) { + nftsWithDifferentMetadata.forEach((elm) => + this.updateNft(elm.nft, elm.newMetadata, userAddress, chainId), + ); + } + } finally { + releaseLock(); } }