fix: update state only when metadata changed after fetch (#4143)

## Explanation

Updating the `updateNftMetadata` function in NftController, to update
the state only when new fetched metadata is different from the metadata
in the state.

On Mobile when Nfts tab is rendered, we are calling addNft function on
all user nfts.
We also do that when a user refreshed the page (by polling down the
tab).
We want to refactor the component and use the `updateNftMetadata`
instead of addNft.

Calling the `addNft` everytime when the component is rendered resulted
in this bug on mobile
MetaMask/metamask-mobile#9196

This one was tricky to reproduce, you will have to:
1- Import an account that has multiple NFTs
2- Create a new account (click on "add new account" button on mobile)
3- Switch back and forth between these two accounts fast and at some
point you will see nfts from the account that has nfts appear on the
account that did not have any nfts.

Still not quite sure what caused this bug now, but i see this
[PR](https://github.com/MetaMask/metamask-mobile/pull/8759/files#diff-c2d4051a35b537d61763a94af5eb70e238542c4b2225554704745a0bcf646dfd)
that has been merged and is part of v7.20.0 release which updates the
"switch account" behavior.

I think what happened is when you are switching to the account that has
nfts, we called addNft function on all the collectibles of accountA and
the addIndividualNft fct is executed. Then when you switched to accountB
(which does not have any nfts) the selectedAddress has switched in the
nftController but we were still trying to add an NFT to state. So we
came here where existingEntry was undefined because accountB does not
have nfts
https://github.com/MetaMask/core/blob/e86b84e6260beb689d95a4bed724d5a55237f35c/packages/assets-controllers/src/NftController.ts#L656
with selectedAddress = accountB and with an nft that does not belong to
accountB and it then executes the update
https://github.com/MetaMask/core/blob/e86b84e6260beb689d95a4bed724d5a55237f35c/packages/assets-controllers/src/NftController.ts#L692
of accountB state.


Refactoring this
[component](https://github.com/MetaMask/metamask-mobile/blob/main/app/components/UI/CollectibleContracts/index.js)
on mobile by removing unecessary useEffects (I think that we had those
useEffects in the first place to fix nft refresh issues, but core
already made updates where its triggering nft refresh when
preferenceController state changes) this will allow removing this
[useEffect](https://github.com/MetaMask/metamask-mobile/blob/9bec1611439841c13c9d75ff033c97b7108f53ba/app/components/UI/CollectibleContracts/index.js#L235)
and this
[one](https://github.com/MetaMask/metamask-mobile/blob/9bec1611439841c13c9d75ff033c97b7108f53ba/app/components/UI/CollectibleContracts/index.js#L271).
The last
[useEffect](https://github.com/MetaMask/metamask-mobile/blob/9bec1611439841c13c9d75ff033c97b7108f53ba/app/components/UI/CollectibleContracts/index.js#L196)
which is triggering addNft, will instead be calling updateNftMetadata
function and will only update the state if necessary.
If a user pulls down the page or goes back and forth between accounts,
we might fetch new nft data but we wont be updating the state.



## References

<!--
Are there any issues that this pull request is tied to? Are there other
links that reviewers should consult to understand these changes better?

For example:

* Fixes #12345
* Related to #67890
-->

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/package-a`

- **FIXED**: Instead of updating state automatically after fetching nft
metadata, we are comparing the fetched metadata with current state and
updating it only when it is different.

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

packages/assets-controllers/src/NftController.test.ts

@@ -3451,6 +3451,116 @@ describe('NftController', () => {
       });
     });
 
+    it('should not update metadata when state nft and fetched nft are the same', async () => {
+      const { nftController } = setupController();
+      const { selectedAddress } = nftController.config;
+      const spy = jest.spyOn(nftController, 'updateNft');
+      const testNetworkClientId = 'sepolia';
+      await nftController.addNft('0xtest', '3', {
+        nftMetadata: {
+          name: 'toto',
+          description: 'description',
+          image: 'image.png',
+          standard: 'ERC721',
+        },
+        networkClientId: testNetworkClientId,
+      });
+
+      sinon
+        .stub(nftController, 'getNftInformation' as keyof typeof nftController)
+        .returns({
+          name: 'toto',
+          image: 'image.png',
+          description: 'description',
+        });
+      const testInputNfts: Nft[] = [
+        {
+          address: '0xtest',
+          description: 'description',
+          favorite: false,
+          image: 'image.png',
+          isCurrentlyOwned: true,
+          name: 'toto',
+          standard: 'ERC721',
+          tokenId: '3',
+        },
+      ];
+
+      await nftController.updateNftMetadata({
+        nfts: testInputNfts,
+        networkClientId: testNetworkClientId,
+      });
+
+      expect(spy).toHaveBeenCalledTimes(0);
+      expect(
+        nftController.state.allNfts[selectedAddress][SEPOLIA.chainId][0],
+      ).toStrictEqual({
+        address: '0xtest',
+        description: 'description',
+        favorite: false,
+        image: 'image.png',
+        isCurrentlyOwned: true,
+        name: 'toto',
+        standard: 'ERC721',
+        tokenId: '3',
+      });
+    });
+
+    it('should trigger update metadata when state nft and fetched nft are not the same', async () => {
+      const { nftController } = setupController();
+      const { selectedAddress } = nftController.config;
+      const spy = jest.spyOn(nftController, 'updateNft');
+      const testNetworkClientId = 'sepolia';
+      await nftController.addNft('0xtest', '3', {
+        nftMetadata: {
+          name: 'toto',
+          description: 'description',
+          image: 'image.png',
+          standard: 'ERC721',
+        },
+        networkClientId: testNetworkClientId,
+      });
+
+      sinon
+        .stub(nftController, 'getNftInformation' as keyof typeof nftController)
+        .returns({
+          name: 'toto',
+          image: 'image-updated.png',
+          description: 'description',
+        });
+      const testInputNfts: Nft[] = [
+        {
+          address: '0xtest',
+          description: 'description',
+          favorite: false,
+          image: 'image.png',
+          isCurrentlyOwned: true,
+          name: 'toto',
+          standard: 'ERC721',
+          tokenId: '3',
+        },
+      ];
+
+      await nftController.updateNftMetadata({
+        nfts: testInputNfts,
+        networkClientId: testNetworkClientId,
+      });
+
+      expect(spy).toHaveBeenCalledTimes(1);
+      expect(
+        nftController.state.allNfts[selectedAddress][SEPOLIA.chainId][0],
+      ).toStrictEqual({
+        address: '0xtest',
+        description: 'description',
+        favorite: false,
+        image: 'image-updated.png',
+        isCurrentlyOwned: true,
+        name: 'toto',
+        standard: 'ERC721',
+        tokenId: '3',
+      });
+    });
+
     it('should not update metadata when calls to fetch metadata fail', async () => {
       const { nftController } = setupController();
       const { selectedAddress } = nftController.config;

packages/assets-controllers/src/NftController.ts

@@ -1416,20 +1416,45 @@ export class NftController extends BaseControllerV1<NftConfig, NftState> {
         };
       }),
     );
+    const successfulNewFetchedNfts = nftMetadataResults.filter(
+      (result): result is PromiseFulfilledResult<NftUpdate> =>
+        result.status === 'fulfilled',
+    );
+    // We want to avoid updating the state if the state and fetched nft info are the same
+    const nftsWithDifferentMetadata: PromiseFulfilledResult<NftUpdate>[] = [];
+    const { allNfts } = this.state;
+    const stateNfts = allNfts[userAddress]?.[chainId] || [];
+
+    successfulNewFetchedNfts.forEach((singleNft) => {
+      const existingEntry: Nft | undefined = stateNfts.find(
+        (nft) =>
+          nft.address.toLowerCase() ===
+            singleNft.value.nft.address.toLowerCase() &&
+          nft.tokenId === singleNft.value.nft.tokenId,
+      );
 
-    nftMetadataResults
-      .filter(
-        (result): result is PromiseFulfilledResult<NftUpdate> =>
-          result.status === 'fulfilled',
-      )
-      .forEach((elm) =>
+      if (existingEntry) {
+        const differentMetadata = compareNftMetadata(
+          singleNft.value.newMetadata,
+          existingEntry,
+        );
+
+        if (differentMetadata) {
+          nftsWithDifferentMetadata.push(singleNft);
+        }
+      }
+    });
+
+    if (nftsWithDifferentMetadata.length !== 0) {
+      nftsWithDifferentMetadata.forEach((elm) =>
         this.updateNft(
           elm.value.nft,
           elm.value.newMetadata,
           userAddress,
           chainId,
         ),
       );
+    }
   }
 
   /**