fix: avoid multiple calls to refresh nft metadata

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);
+    });
   });
 });

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();
     }
   }