From 68f91cd5d215271eefbb24674268a8d5ab1fac35 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 23 May 2024 13:34:49 +0200 Subject: [PATCH 01/17] feat: migrate NftController to BaseControllerV2 --- .../src/NftController.test.ts | 172 ++-- .../assets-controllers/src/NftController.ts | 773 +++++++++--------- .../src/NftDetectionController.test.ts | 15 +- .../src/NftDetectionController.ts | 10 +- 4 files changed, 522 insertions(+), 448 deletions(-) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 8e2de092d78..71434698e85 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -43,7 +43,11 @@ import { } from '../../network-controller/tests/helpers'; import { getFormattedIpfsUrl } from './assetsUtil'; import { Source } from './constants'; -import type { Nft, NftControllerMessenger } from './NftController'; +import type { + Nft, + NftControllerState, + NftControllerMessenger, +} from './NftController'; import { NftController } from './NftController'; const CRYPTOPUNK_ADDRESS = '0xb47e3cd837dDF8e4c57F05d70Ab865de6e193BBB'; @@ -868,11 +872,14 @@ describe('NftController', () => { }); const acceptedRequest = new Promise((resolve) => { - nftController.subscribe((state) => { - if (state.allNfts?.[SECOND_OWNER_ADDRESS]?.[GOERLI.chainId]) { - resolve(); - } - }); + messenger.subscribe( + 'NftController:stateChange', + (state: NftControllerState) => { + if (state.allNfts?.[SECOND_OWNER_ADDRESS]?.[GOERLI.chainId]) { + resolve(); + } + }, + ); }); // check that the NFT is not in state to begin with @@ -964,11 +971,14 @@ describe('NftController', () => { }); const acceptedRequest = new Promise((resolve) => { - nftController.subscribe((state) => { - if (state.allNfts?.[OWNER_ADDRESS]?.[GOERLI.chainId].length) { - resolve(); - } - }); + messenger.subscribe( + 'NftController:stateChange', + (state: NftControllerState) => { + if (state.allNfts?.[OWNER_ADDRESS]?.[GOERLI.chainId].length) { + resolve(); + } + }, + ); }); // check that the NFT is not in state to begin with @@ -1052,7 +1062,7 @@ describe('NftController', () => { }, }); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x01', '1', { nftMetadata: { name: 'name', @@ -1153,7 +1163,7 @@ describe('NftController', () => { it('should add NFT by selected address', async () => { const { nftController, triggerPreferencesStateChange } = setupController(); - const { chainId } = nftController.config; + const { chainId } = nftController.getConfig(); const firstAddress = '0x123'; const secondAddress = '0x321'; @@ -1194,7 +1204,7 @@ describe('NftController', () => { it('should update NFT if image is different', async () => { const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x01', '1', { nftMetadata: { @@ -1245,7 +1255,7 @@ describe('NftController', () => { it('should not duplicate NFT nor NFT contract if already added', async () => { const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x01', '1', { nftMetadata: { name: 'name', @@ -1287,7 +1297,7 @@ describe('NftController', () => { }, }); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x01', '1'); expect( nftController.state.allNfts[selectedAddress][chainId][0], @@ -1343,7 +1353,7 @@ describe('NftController', () => { description: 'Kudos Description (directly from tokenURI)', }); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft(ERC721_KUDOSADDRESS, ERC721_KUDOS_TOKEN_ID); @@ -1396,7 +1406,7 @@ describe('NftController', () => { image: 'image (directly from tokenURI)', animation_url: null, }); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft(ERC1155_NFT_ADDRESS, ERC1155_NFT_ID); expect( @@ -1437,7 +1447,7 @@ describe('NftController', () => { name: 'Kudos Name (directly from tokenURI)', description: 'Kudos Description (directly from tokenURI)', }); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); sinon // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -1473,7 +1483,7 @@ describe('NftController', () => { it('should add NFT by provider type', async () => { const { nftController, changeNetwork } = setupController(); - const { selectedAddress } = nftController.config; + const { selectedAddress } = nftController.getConfig(); sinon // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -1509,7 +1519,7 @@ describe('NftController', () => { onNftAdded: mockOnNftAdded, }, }); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any sinon.stub(nftController, 'getNftContractInformation' as any).returns({ @@ -1688,7 +1698,7 @@ describe('NftController', () => { image_url: 'Kudos logo (from proxy API)', }); */ - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft( '0x6EbeAf8e8E946F0716E6533A6f2cefc83f60e8Ab', '123', @@ -1771,7 +1781,7 @@ describe('NftController', () => { ) .replyWithError(new Error('Failed to fetch')); - const { selectedAddress } = nftController.config; + const { selectedAddress } = nftController.getConfig(); await nftController.addNft( '0x6EbeAf8e8E946F0716E6533A6f2cefc83f60e8Ab', @@ -1793,7 +1803,7 @@ describe('NftController', () => { it('should not add duplicate NFTs to the ignoredNfts list', async () => { const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x01', '1', { nftMetadata: { @@ -1865,10 +1875,10 @@ describe('NftController', () => { .mockRejectedValue(new Error('Not an ERC1155 token')), }, }); - nftController.configure({ + nftController.setConfig({ ipfsGateway: IPFS_DEFAULT_GATEWAY_URL, }); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, @@ -1907,7 +1917,7 @@ describe('NftController', () => { ) .replyWithError(new Error('Failed to fetch')); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft(ERC721_NFT_ADDRESS, ERC721_NFT_ID); @@ -2170,7 +2180,7 @@ describe('NftController', () => { setupController(); const firstAddress = '0x123'; const secondAddress = '0x321'; - const { chainId } = nftController.config; + const { chainId } = nftController.getConfig(); // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -2345,7 +2355,7 @@ describe('NftController', () => { describe('removeNft', () => { it('should remove NFT and NFT contract', async () => { const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x01', '1', { nftMetadata: { @@ -2367,7 +2377,7 @@ describe('NftController', () => { it('should not remove NFT contract if NFT still exists', async () => { const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x01', '1', { nftMetadata: { @@ -2399,7 +2409,7 @@ describe('NftController', () => { it('should remove NFT by selected address', async () => { const { nftController, triggerPreferencesStateChange } = setupController(); - const { chainId } = nftController.config; + const { chainId } = nftController.getConfig(); sinon // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -2443,7 +2453,7 @@ describe('NftController', () => { it('should remove NFT by provider type', async () => { const { nftController, changeNetwork } = setupController(); - const { selectedAddress } = nftController.config; + const { selectedAddress } = nftController.getConfig(); sinon // TODO: Replace `any` with type @@ -2535,7 +2545,7 @@ describe('NftController', () => { it('should be able to clear the ignoredNfts list', async () => { const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x02', '1', { nftMetadata: { @@ -2706,7 +2716,7 @@ describe('NftController', () => { .stub(nftController, 'getNftURIAndStandard' as any) .returns(['ipfs://*', ERC1155]); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft(ERC1155_NFT_ADDRESS, ERC1155_NFT_ID); @@ -2729,7 +2739,7 @@ describe('NftController', () => { describe('updateNftFavoriteStatus', () => { it('should not set NFT as favorite if nft not found', async () => { const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2754,7 +2764,7 @@ describe('NftController', () => { }); it('should set NFT as favorite', async () => { const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2780,7 +2790,7 @@ describe('NftController', () => { it('should set NFT as favorite and then unset it', async () => { const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2822,7 +2832,7 @@ describe('NftController', () => { it('should keep the favorite status as true after updating metadata', async () => { const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2879,7 +2889,7 @@ describe('NftController', () => { it('should keep the favorite status as false after updating metadata', async () => { const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2996,7 +3006,7 @@ describe('NftController', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any sinon.stub(nftController, 'isNftOwner' as any).returns(false); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x02', '1', { nftMetadata: { name: 'name', @@ -3025,7 +3035,7 @@ describe('NftController', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any sinon.stub(nftController, 'isNftOwner' as any).returns(true); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x02', '1', { nftMetadata: { name: 'name', @@ -3056,7 +3066,7 @@ describe('NftController', () => { .stub(nftController, 'isNftOwner' as any) .throws(new Error('Unable to verify ownership')); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x02', '1', { nftMetadata: { name: 'name', @@ -3090,7 +3100,7 @@ describe('NftController', () => { }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x02', '1', { nftMetadata: { name: 'name', @@ -3132,7 +3142,7 @@ describe('NftController', () => { describe('checkAndUpdateSingleNftOwnershipStatus', () => { it('should check whether the passed NFT is still owned by the the current selectedAddress/chainId combination and update its isCurrentlyOwned property in state if batch is false and isNftOwner returns false', async () => { const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); const nft = { address: '0x02', tokenId: '1', @@ -3166,7 +3176,7 @@ describe('NftController', () => { it('should check whether the passed NFT is still owned by the the current selectedAddress/chainId combination and return the updated NFT object without updating state if batch is true', async () => { const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); const nft = { address: '0x02', tokenId: '1', @@ -3212,7 +3222,7 @@ describe('NftController', () => { }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); const nft = { address: '0x02', tokenId: '1', @@ -3265,7 +3275,7 @@ describe('NftController', () => { }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); - const { selectedAddress, chainId } = nftController.config; + const { selectedAddress, chainId } = nftController.getConfig(); const nft = { address: '0x02', tokenId: '1', @@ -3329,10 +3339,11 @@ describe('NftController', () => { standard: 'standard', favorite: false, }; - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.config; it('should return null if the NFT does not exist in the state', async () => { + const { nftController } = setupController(); + const { selectedAddress, chainId } = nftController.getConfig(); + expect( nftController.findNftByAddressAndTokenId( mockNft.address, @@ -3344,9 +3355,16 @@ describe('NftController', () => { }); it('should return the NFT by the address and tokenId', () => { - nftController.state.allNfts = { - [selectedAddress]: { [chainId]: [mockNft] }, - }; + const { nftController } = setupController({ + options: { + state: { + allNfts: { + [OWNER_ADDRESS]: { [ChainId.mainnet]: [mockNft] }, + }, + }, + }, + }); + const { selectedAddress, chainId } = nftController.getConfig(); expect( nftController.findNftByAddressAndTokenId( @@ -3360,8 +3378,6 @@ describe('NftController', () => { }); describe('updateNftByAddressAndTokenId', () => { - const { nftController } = setupController(); - const mockTransactionId = '60d36710-b150-11ec-8a49-c377fbd05e27'; const mockNft = { address: '0x02', @@ -3384,12 +3400,17 @@ describe('NftController', () => { transactionId: mockTransactionId, }; - const { selectedAddress, chainId } = nftController.config; - it('should update the NFT if the NFT exist', async () => { - nftController.state.allNfts = { - [selectedAddress]: { [chainId]: [mockNft] }, - }; + const { nftController } = setupController({ + options: { + state: { + allNfts: { + [OWNER_ADDRESS]: { [ChainId.mainnet]: [mockNft] }, + }, + }, + }, + }); + const { selectedAddress, chainId } = nftController.getConfig(); nftController.updateNft( mockNft, @@ -3406,6 +3427,9 @@ describe('NftController', () => { }); it('should return undefined if the NFT does not exist', () => { + const { nftController } = setupController(); + const { selectedAddress, chainId } = nftController.getConfig(); + expect( nftController.updateNft( mockNft, @@ -3420,8 +3444,6 @@ describe('NftController', () => { }); describe('resetNftTransactionStatusByTransactionId', () => { - const { nftController } = setupController(); - const mockTransactionId = '60d36710-b150-11ec-8a49-c377fbd05e27'; const nonExistTransactionId = '0123'; @@ -3436,9 +3458,10 @@ describe('NftController', () => { transactionId: mockTransactionId, }; - const { selectedAddress, chainId } = nftController.config; - it('should not update any NFT state and should return false when passed a transaction id that does not match that of any NFT', async () => { + const { nftController } = setupController(); + const { selectedAddress, chainId } = nftController.getConfig(); + expect( nftController.resetNftTransactionStatusByTransactionId( nonExistTransactionId, @@ -3449,9 +3472,16 @@ describe('NftController', () => { }); it('should set the transaction id of an NFT in state to undefined, and return true when it has successfully updated this state', async () => { - nftController.state.allNfts = { - [selectedAddress]: { [chainId]: [mockNft] }, - }; + const { nftController } = setupController({ + options: { + state: { + allNfts: { + [OWNER_ADDRESS]: { [ChainId.mainnet]: [mockNft] }, + }, + }, + }, + }); + const { selectedAddress, chainId } = nftController.getConfig(); expect( nftController.state.allNfts[selectedAddress][chainId][0].transactionId, @@ -3474,7 +3504,7 @@ describe('NftController', () => { describe('updateNftMetadata', () => { it('should update Nft metadata successfully', async () => { const { nftController } = setupController(); - const { selectedAddress } = nftController.config; + const { selectedAddress } = nftController.getConfig(); const spy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; await nftController.addNft('0xtest', '3', { @@ -3526,7 +3556,7 @@ 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 { selectedAddress } = nftController.getConfig(); const spy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; await nftController.addNft('0xtest', '3', { @@ -3581,7 +3611,7 @@ describe('NftController', () => { it('should trigger update metadata when state nft and fetched nft are not the same', async () => { const { nftController } = setupController(); - const { selectedAddress } = nftController.config; + const { selectedAddress } = nftController.getConfig(); const spy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; await nftController.addNft('0xtest', '3', { @@ -3636,7 +3666,7 @@ describe('NftController', () => { it('should not update metadata when calls to fetch metadata fail', async () => { const { nftController } = setupController(); - const { selectedAddress } = nftController.config; + const { selectedAddress } = nftController.getConfig(); const spy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; await nftController.addNft('0xtest', '3', { @@ -3687,7 +3717,7 @@ describe('NftController', () => { it('should update metadata when some calls to fetch metadata succeed', async () => { const { nftController } = setupController(); - const { selectedAddress } = nftController.config; + const { selectedAddress } = nftController.getConfig(); const spy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; // Add nfts diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index ff712bf8bf3..0c3d947a7f7 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -1,11 +1,10 @@ import { isAddress } from '@ethersproject/address'; import type { AddApprovalRequest } from '@metamask/approval-controller'; import type { - BaseConfig, - BaseState, RestrictedControllerMessenger, + ControllerStateChangeEvent, } from '@metamask/base-controller'; -import { BaseControllerV1 } from '@metamask/base-controller'; +import { BaseController } from '@metamask/base-controller'; import { safelyExecute, handleFetch, @@ -30,7 +29,6 @@ import type { Hex } from '@metamask/utils'; import { remove0x } from '@metamask/utils'; import { Mutex } from 'async-mutex'; import BN from 'bn.js'; -import { EventEmitter } from 'events'; import { v4 as random } from 'uuid'; import type { AssetsContractController } from './AssetsContractController'; @@ -76,14 +74,12 @@ 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 */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface Nft extends NftMetadata { - tokenId: string; - address: string; - isCurrentlyOwned?: boolean; -} +export type Nft = + | { + tokenId: string; + address: string; + isCurrentlyOwned?: boolean; + } & NftMetadata; type NftUpdate = { nft: Nft; @@ -105,10 +101,7 @@ type NftUpdate = { * @property schemaName - The schema followed by the contract, it could be `ERC721` or `ERC1155` * @property externalLink - External link containing additional information */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface NftContract { +export type NftContract = { name?: string; logo?: string; address: string; @@ -119,7 +112,7 @@ export interface NftContract { createdDate?: string; schemaName?: string; externalLink?: string; -} +}; /** * @type NftMetadata @@ -139,10 +132,7 @@ export interface NftContract { * @property creator - The NFT owner information object * @property standard - NFT standard name for the NFT, e.g., ERC-721 or ERC-1155 */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface NftMetadata { +export type NftMetadata = { name: string | null; description: string | null; image: string | null; @@ -164,7 +154,7 @@ export interface NftMetadata { attributes?: Attributes; lastSale?: LastSale; rarityRank?: string; -} +}; /** * @type NftConfig @@ -172,47 +162,97 @@ export interface NftMetadata { * NFT controller configuration * @property selectedAddress - Vault selected address */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface NftConfig extends BaseConfig { +export type NftConfig = { selectedAddress: string; chainId: Hex; ipfsGateway: string; openSeaEnabled: boolean; useIPFSSubdomains: boolean; isIpfsGatewayEnabled: boolean; -} +}; /** - * @type NftState + * @type NftControllerState * * NFT controller state * @property allNftContracts - Object containing NFT contract information * @property allNfts - Object containing NFTs per account and network * @property ignoredNfts - List of NFTs that should be ignored */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface NftState extends BaseState { +export type NftControllerState = { allNftContracts: { - [key: string]: { [chainId: Hex]: NftContract[] }; + [key: string]: { + [chainId: Hex]: NftContract[]; + }; + }; + allNfts: { + [key: string]: { + [chainId: Hex]: Nft[]; + }; }; - allNfts: { [key: string]: { [chainId: Hex]: Nft[] } }; ignoredNfts: Nft[]; -} +}; + +/** + * Creates an NftController instance. + * + * @param options - The controller options. + * @param options.chainId - The chain ID of the current network. + * @param options.onPreferencesStateChange - Allows subscribing to preference controller state changes. + * @param options.onNetworkStateChange - Allows subscribing to network controller state changes. + * @param options.getERC721AssetName - Gets the name of the asset at the given address. + * @param options.getERC721AssetSymbol - Gets the symbol of the asset at the given address. + * @param options.getERC721TokenURI - Gets the URI of the ERC721 token at the given address, with the given ID. + * @param options.getERC721OwnerOf - Get the owner of a ERC-721 NFT. + * @param options.getERC1155BalanceOf - Gets balance of a ERC-1155 NFT. + * @param options.getERC1155TokenURI - Gets the URI of the ERC1155 token at the given address, with the given ID. + * @param options.getNetworkClientById - Gets the network client for the given networkClientId. + * @param options.onNftAdded - Callback that is called when an NFT is added. Currently used pass data + * for tracking the NFT added event. + * @param options.messenger - The controller messenger. + * @param options.config - Initial options used to configure this controller. + * @param options.state - Initial state to set on this controller. + */ +export type NftConftrollerOptions = { + chainId: Hex; + onPreferencesStateChange: ( + listener: (preferencesState: PreferencesState) => void, + ) => void; + onNetworkStateChange: ( + listener: (networkState: NetworkState) => void, + ) => void; + getERC721AssetName: AssetsContractController['getERC721AssetName']; + getERC721AssetSymbol: AssetsContractController['getERC721AssetSymbol']; + getERC721TokenURI: AssetsContractController['getERC721TokenURI']; + getERC721OwnerOf: AssetsContractController['getERC721OwnerOf']; + getERC1155BalanceOf: AssetsContractController['getERC1155BalanceOf']; + getERC1155TokenURI: AssetsContractController['getERC1155TokenURI']; + getNetworkClientById: NetworkController['getNetworkClientById']; + onNftAdded?: (data: { + address: string; + symbol: string | undefined; + tokenId: string; + standard: string | null; + source: string; + }) => void; + messenger: NftControllerMessenger; + config?: Partial; + state?: Partial; +}; + +const metadata = { + allNftContracts: { persist: true, anonymous: false }, + allNfts: { persist: true, anonymous: false }, + ignoredNfts: { persist: true, anonymous: false }, +}; const ALL_NFTS_STATE_KEY = 'allNfts'; const ALL_NFTS_CONTRACTS_STATE_KEY = 'allNftContracts'; -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -interface NftAsset { +type NftAsset = { address: string; tokenId: string; -} +}; /** * The name of the {@link NftController}. @@ -226,37 +266,175 @@ type AllowedActions = | AddApprovalRequest | NetworkControllerGetNetworkClientByIdAction; +export type NftControllerStateChangeEvent = ControllerStateChangeEvent< + typeof controllerName, + NftControllerState +>; + +export type NFtControllerEvents = NftControllerStateChangeEvent; + /** * The messenger of the {@link NftController}. */ export type NftControllerMessenger = RestrictedControllerMessenger< typeof controllerName, AllowedActions, - never, + NFtControllerEvents, AllowedActions['type'], never >; -export const getDefaultNftState = (): NftState => { - return { - allNftContracts: {}, - allNfts: {}, - ignoredNfts: [], - }; +export const defaultNftControllerState: NftControllerState = { + allNftContracts: {}, + allNfts: {}, + ignoredNfts: [], }; /** * Controller that stores assets and exposes convenience methods */ -export class NftController extends BaseControllerV1 { - private readonly mutex = new Mutex(); +export class NftController extends BaseController< + typeof controllerName, + NftControllerState, + NftControllerMessenger +> { + readonly #mutex = new Mutex(); + + /** + * Optional API key to use with opensea + */ + openSeaApiKey?: string; + + #config: NftConfig; + + readonly #getERC721AssetName: AssetsContractController['getERC721AssetName']; + + readonly #getERC721AssetSymbol: AssetsContractController['getERC721AssetSymbol']; + + readonly #getERC721TokenURI: AssetsContractController['getERC721TokenURI']; + + readonly #getERC721OwnerOf: AssetsContractController['getERC721OwnerOf']; + + readonly #getERC1155BalanceOf: AssetsContractController['getERC1155BalanceOf']; + + readonly #getERC1155TokenURI: AssetsContractController['getERC1155TokenURI']; + + readonly #getNetworkClientById: NetworkController['getNetworkClientById']; + + readonly #onNftAdded?: (data: { + address: string; + symbol: string | undefined; + tokenId: string; + standard: string | null; + source: Source; + }) => void; + + constructor({ + chainId: initialChainId, + onPreferencesStateChange, + onNetworkStateChange, + getERC721AssetName, + getERC721AssetSymbol, + getERC721TokenURI, + getERC721OwnerOf, + getERC1155BalanceOf, + getERC1155TokenURI, + getNetworkClientById, + onNftAdded, + messenger, + config = {}, + state = {}, + }: NftConftrollerOptions) { + super({ + name: controllerName, + metadata, + messenger, + state: { + ...defaultNftControllerState, + ...state, + }, + }); + this.#config = { + selectedAddress: '', + chainId: initialChainId, + ipfsGateway: IPFS_DEFAULT_GATEWAY_URL, + openSeaEnabled: false, + useIPFSSubdomains: true, + isIpfsGatewayEnabled: true, + ...config, + }; - private readonly messagingSystem: NftControllerMessenger; + this.#getERC721AssetName = getERC721AssetName; + this.#getERC721AssetSymbol = getERC721AssetSymbol; + this.#getERC721TokenURI = getERC721TokenURI; + this.#getERC721OwnerOf = getERC721OwnerOf; + this.#getERC1155BalanceOf = getERC1155BalanceOf; + this.#getERC1155TokenURI = getERC1155TokenURI; + this.#getNetworkClientById = getNetworkClientById; + this.#onNftAdded = onNftAdded; + this.messagingSystem = messenger; + + onPreferencesStateChange( + async ({ + selectedAddress, + ipfsGateway, + openSeaEnabled, + isIpfsGatewayEnabled, + }) => { + this.#config = { + ...this.#config, + selectedAddress, + ipfsGateway, + openSeaEnabled, + isIpfsGatewayEnabled, + }; + + const needsUpdateNftMetadata = + (isIpfsGatewayEnabled && ipfsGateway !== '') || openSeaEnabled; + + if (needsUpdateNftMetadata) { + const { chainId } = this.#config; + const nfts: Nft[] = + this.state.allNfts[selectedAddress]?.[chainId] ?? []; + // filter only nfts + const nftsToUpdate = nfts.filter( + (singleNft) => + !singleNft.name && !singleNft.description && !singleNft.image, + ); + if (nftsToUpdate.length !== 0) { + await this.updateNftMetadata({ + nfts: nftsToUpdate, + userAddress: selectedAddress, + }); + } + } + }, + ); + + onNetworkStateChange(({ selectedNetworkClientId }) => { + const selectedNetworkClient = getNetworkClientById( + selectedNetworkClientId, + ); + const { chainId } = selectedNetworkClient.configuration; + this.#config.chainId = chainId; + }); + } getNftApi() { return `${NFT_API_BASE_URL}/tokens`; } + getConfig(): Readonly { + return this.#config; + } + + setConfig(config: Partial): void { + this.#config = { + ...this.#config, + ...config, + }; + } + /** * Helper method to update nested state for allNfts and allNftContracts. * @@ -266,24 +444,25 @@ export class NftController extends BaseControllerV1 { * @param passedConfig.userAddress - the address passed through the NFT detection flow to ensure assets are stored to the correct account * @param passedConfig.chainId - the chainId passed through the NFT detection flow to ensure assets are stored to the correct account */ - private updateNestedNftState( - newCollection: Nft[] | NftContract[], - baseStateKey: 'allNfts' | 'allNftContracts', - { userAddress, chainId }: { userAddress: string; chainId: Hex }, + #updateNestedNftState< + T extends 'allNfts' | 'allNftContracts', + U extends T extends 'allNfts' ? Nft[] : NftContract[], + >( + newCollection: U, + baseStateKey: T, + { userAddress, chainId }: { userAddress: string; chainId: string }, ) { - const { [baseStateKey]: oldState } = this.state; - - const addressState = oldState[userAddress]; - const newAddressState = { - ...addressState, - ...{ [chainId]: newCollection }, - }; - const newState = { - ...oldState, - ...{ [userAddress]: newAddressState }, - }; - this.update({ - [baseStateKey]: newState, + this.update((state) => { + const oldState = state[baseStateKey]; + const addressState = oldState[userAddress] || {}; + const newAddressState = { + ...addressState, + [chainId]: newCollection, + }; + state[baseStateKey] = { + ...oldState, + [userAddress]: newAddressState, + }; }); } @@ -374,13 +553,13 @@ export class NftController extends BaseControllerV1 { * @param networkClientId - The networkClientId that can be used to identify the network client to use for this request. * @returns Promise resolving to the current NFT name and image. */ - private async getNftInformationFromTokenURI( + async #getNftInformationFromTokenURI( contractAddress: string, tokenId: string, networkClientId?: NetworkClientId, ): Promise { const { ipfsGateway, useIPFSSubdomains, isIpfsGatewayEnabled } = - this.config; + this.#config; const result = await this.getNftURIAndStandard( contractAddress, tokenId, @@ -402,7 +581,7 @@ export class NftController extends BaseControllerV1 { }; } - const isDisplayNFTMediaToggleEnabled = this.config.openSeaEnabled; + const isDisplayNFTMediaToggleEnabled = this.#config.openSeaEnabled; if (!hasIpfsTokenURI && !isDisplayNFTMediaToggleEnabled) { return { image: null, @@ -453,14 +632,14 @@ export class NftController extends BaseControllerV1 { * @param networkClientId - The networkClientId that can be used to identify the network client to use for this request. * @returns Promise resolving NFT uri and token standard. */ - private async getNftURIAndStandard( + async getNftURIAndStandard( contractAddress: string, tokenId: string, networkClientId?: NetworkClientId, ): Promise<[string, string]> { // try ERC721 uri try { - const uri = await this.getERC721TokenURI( + const uri = await this.#getERC721TokenURI( contractAddress, tokenId, networkClientId, @@ -472,7 +651,7 @@ export class NftController extends BaseControllerV1 { // try ERC1155 uri try { - const tokenURI = await this.getERC1155TokenURI( + const tokenURI = await this.#getERC1155TokenURI( contractAddress, tokenId, networkClientId, @@ -512,18 +691,18 @@ export class NftController extends BaseControllerV1 { tokenId: string, networkClientId?: NetworkClientId, ): Promise { - const chainId = this.getCorrectChainId({ + const chainId = this.#getCorrectChainId({ networkClientId, }); const [blockchainMetadata, nftApiMetadata] = await Promise.all([ safelyExecute(() => - this.getNftInformationFromTokenURI( + this.#getNftInformationFromTokenURI( contractAddress, tokenId, networkClientId, ), ), - this.config.openSeaEnabled && chainId === '0x1' + this.#config.openSeaEnabled && chainId === '0x1' ? safelyExecute(() => this.getNftInformationFromApi(contractAddress, tokenId), ) @@ -548,7 +727,7 @@ export class NftController extends BaseControllerV1 { * @param networkClientId - The networkClientId that can be used to identify the network client to use for this request. * @returns Promise resolving to the current NFT name and image. */ - private async getNftContractInformationFromContract( + async #getNftContractInformationFromContract( contractAddress: string, networkClientId?: NetworkClientId, ): Promise< @@ -557,8 +736,8 @@ export class NftController extends BaseControllerV1 { Pick > { const [name, symbol] = await Promise.all([ - this.getERC721AssetName(contractAddress, networkClientId), - this.getERC721AssetSymbol(contractAddress, networkClientId), + this.#getERC721AssetName(contractAddress, networkClientId), + this.#getERC721AssetSymbol(contractAddress, networkClientId), ]); return { @@ -586,7 +765,7 @@ export class NftController extends BaseControllerV1 { Pick > { const blockchainContractData = await safelyExecute(() => - this.getNftContractInformationFromContract( + this.#getNftContractInformationFromContract( contractAddress, networkClientId, ), @@ -639,7 +818,7 @@ export class NftController extends BaseControllerV1 { * @param source - Whether the NFT was detected, added manually or suggested by a dapp. * @returns Promise resolving to the current NFT list. */ - private async addIndividualNft( + async #addIndividualNft( tokenAddress: string, tokenId: string, nftMetadata: NftMetadata, @@ -647,18 +826,17 @@ export class NftController extends BaseControllerV1 { chainId: Hex, userAddress: string, source: Source, - ): Promise { - // TODO: Remove unused return - const releaseLock = await this.mutex.acquire(); + ): Promise { + const releaseLock = await this.#mutex.acquire(); try { - tokenAddress = toChecksumHexAddress(tokenAddress); + const checksumHexAddress = toChecksumHexAddress(tokenAddress); const { allNfts } = this.state; - const nfts = allNfts[userAddress]?.[chainId] || []; + const nfts = [...(allNfts[userAddress]?.[chainId] || [])]; - const existingEntry: Nft | undefined = nfts.find( + const existingEntry = nfts.find( (nft) => - nft.address.toLowerCase() === tokenAddress.toLowerCase() && + nft.address.toLowerCase() === checksumHexAddress.toLowerCase() && nft.tokenId === tokenId, ); @@ -667,46 +845,49 @@ export class NftController extends BaseControllerV1 { nftMetadata, existingEntry, ); - if (differentMetadata || !existingEntry.isCurrentlyOwned) { - // TODO: Switch to indexToUpdate - const indexToRemove = nfts.findIndex( - (nft) => - nft.address.toLowerCase() === tokenAddress.toLowerCase() && - nft.tokenId === tokenId, - ); - /* istanbul ignore next */ - if (indexToRemove !== -1) { - nfts.splice(indexToRemove, 1); - } - } else { - return nfts; + + if (!differentMetadata && existingEntry.isCurrentlyOwned) { + return; } - } - const newEntry: Nft = { - address: tokenAddress, - tokenId, - favorite: existingEntry?.favorite || false, - isCurrentlyOwned: true, - ...nftMetadata, - }; + const indexToUpdate = nfts.findIndex( + (nft) => + nft.address.toLowerCase() === checksumHexAddress.toLowerCase() && + nft.tokenId === tokenId, + ); + + if (indexToUpdate !== -1) { + nfts[indexToUpdate] = { + ...existingEntry, + ...nftMetadata, + }; + } + } else { + const newEntry: Nft = { + address: checksumHexAddress, + tokenId, + favorite: false, + isCurrentlyOwned: true, + ...nftMetadata, + }; - const newNfts = [...nfts, newEntry]; - this.updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { + nfts.push(newEntry); + } + + this.#updateNestedNftState(nfts, 'allNfts', { chainId, userAddress, }); - if (this.onNftAdded) { - this.onNftAdded({ - address: tokenAddress, + if (this.#onNftAdded) { + this.#onNftAdded({ + address: checksumHexAddress, symbol: nftContract.symbol, tokenId: tokenId.toString(), standard: nftMetadata.standard, source, }); } - return newNfts; } finally { releaseLock(); } @@ -723,7 +904,7 @@ export class NftController extends BaseControllerV1 { * @param options.source - Whether the NFT was detected, added manually or suggested by a dapp. * @returns Promise resolving to the current NFT contracts list. */ - private async addNftContract({ + async #addNftContract({ tokenAddress, userAddress, networkClientId, @@ -736,11 +917,11 @@ export class NftController extends BaseControllerV1 { networkClientId?: NetworkClientId; source?: Source; }): Promise { - const releaseLock = await this.mutex.acquire(); + const releaseLock = await this.#mutex.acquire(); try { - tokenAddress = toChecksumHexAddress(tokenAddress); + const checksumHexAddress = toChecksumHexAddress(tokenAddress); const { allNftContracts } = this.state; - const chainId = this.getCorrectChainId({ + const chainId = this.#getCorrectChainId({ networkClientId, }); @@ -748,7 +929,8 @@ export class NftController extends BaseControllerV1 { const existingEntry = nftContracts.find( (nftContract) => - nftContract.address.toLowerCase() === tokenAddress.toLowerCase(), + nftContract.address.toLowerCase() === + checksumHexAddress.toLowerCase(), ); if (existingEntry) { return nftContracts; @@ -758,7 +940,7 @@ export class NftController extends BaseControllerV1 { // will be fixed once detection uses networkClientIds // get name and symbol if ERC721 then put together the metadata const contractInformation = await this.getNftContractInformation( - tokenAddress, + checksumHexAddress, nftMetadata, networkClientId, ); @@ -791,7 +973,7 @@ export class NftController extends BaseControllerV1 { /* istanbul ignore next */ const newEntry: NftContract = Object.assign( {}, - { address: tokenAddress }, + { address: checksumHexAddress }, description && { description }, name && { name }, image_url && { logo: image_url }, @@ -804,10 +986,14 @@ export class NftController extends BaseControllerV1 { external_link && { externalLink: external_link }, ); const newNftContracts = [...nftContracts, newEntry]; - this.updateNestedNftState(newNftContracts, ALL_NFTS_CONTRACTS_STATE_KEY, { - chainId, - userAddress, - }); + this.#updateNestedNftState( + newNftContracts, + ALL_NFTS_CONTRACTS_STATE_KEY, + { + chainId, + userAddress, + }, + ); return newNftContracts; } finally { @@ -824,7 +1010,7 @@ export class NftController extends BaseControllerV1 { * @param options.chainId - The chainId of the network where the NFT is being removed. * @param options.userAddress - The address of the account where the NFT is being removed. */ - private removeAndIgnoreIndividualNft( + #removeAndIgnoreIndividualNft( address: string, tokenId: string, { @@ -835,17 +1021,17 @@ export class NftController extends BaseControllerV1 { userAddress: string; }, ) { - address = toChecksumHexAddress(address); + const checksumHexAddress = toChecksumHexAddress(address); const { allNfts, ignoredNfts } = this.state; const newIgnoredNfts = [...ignoredNfts]; const nfts = allNfts[userAddress]?.[chainId] || []; const newNfts = nfts.filter((nft) => { if ( - nft.address.toLowerCase() === address.toLowerCase() && + nft.address.toLowerCase() === checksumHexAddress.toLowerCase() && nft.tokenId === tokenId ) { const alreadyIgnored = newIgnoredNfts.find( - (c) => c.address === address && c.tokenId === tokenId, + (c) => c.address === checksumHexAddress && c.tokenId === tokenId, ); !alreadyIgnored && newIgnoredNfts.push(nft); return false; @@ -853,13 +1039,13 @@ export class NftController extends BaseControllerV1 { return true; }); - this.updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { + this.#updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { userAddress, chainId, }); - this.update({ - ignoredNfts: newIgnoredNfts, + this.update((state) => { + state.ignoredNfts = newIgnoredNfts; }); } @@ -872,22 +1058,22 @@ export class NftController extends BaseControllerV1 { * @param options.chainId - The chainId of the network where the NFT is being removed. * @param options.userAddress - The address of the account where the NFT is being removed. */ - private removeIndividualNft( + #removeIndividualNft( address: string, tokenId: string, { chainId, userAddress }: { chainId: Hex; userAddress: string }, ) { - address = toChecksumHexAddress(address); + const checksumHexAddress = toChecksumHexAddress(address); const { allNfts } = this.state; const nfts = allNfts[userAddress]?.[chainId] || []; const newNfts = nfts.filter( (nft) => !( - nft.address.toLowerCase() === address.toLowerCase() && + nft.address.toLowerCase() === checksumHexAddress.toLowerCase() && nft.tokenId === tokenId ), ); - this.updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { + this.#updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { userAddress, chainId, }); @@ -902,19 +1088,21 @@ export class NftController extends BaseControllerV1 { * @param options.userAddress - The address of the account where the NFT is being removed. * @returns Promise resolving to the current NFT contracts list. */ - private removeNftContract( + #removeNftContract( address: string, { chainId, userAddress }: { chainId: Hex; userAddress: string }, ): NftContract[] { - address = toChecksumHexAddress(address); + const checksumHexAddress = toChecksumHexAddress(address); const { allNftContracts } = this.state; const nftContracts = allNftContracts[userAddress]?.[chainId] || []; const newNftContracts = nftContracts.filter( (nftContract) => - !(nftContract.address.toLowerCase() === address.toLowerCase()), + !( + nftContract.address.toLowerCase() === checksumHexAddress.toLowerCase() + ), ); - this.updateNestedNftState(newNftContracts, ALL_NFTS_CONTRACTS_STATE_KEY, { + this.#updateNestedNftState(newNftContracts, ALL_NFTS_CONTRACTS_STATE_KEY, { chainId, userAddress, }); @@ -922,173 +1110,7 @@ export class NftController extends BaseControllerV1 { return newNftContracts; } - /** - * EventEmitter instance used to listen to specific EIP747 events - */ - hub = new EventEmitter(); - - /** - * Optional API key to use with opensea - */ - openSeaApiKey?: string; - - /** - * Name of this controller used during composition - */ - override name = 'NftController'; - - private readonly getERC721AssetName: AssetsContractController['getERC721AssetName']; - - private readonly getERC721AssetSymbol: AssetsContractController['getERC721AssetSymbol']; - - private readonly getERC721TokenURI: AssetsContractController['getERC721TokenURI']; - - private readonly getERC721OwnerOf: AssetsContractController['getERC721OwnerOf']; - - private readonly getERC1155BalanceOf: AssetsContractController['getERC1155BalanceOf']; - - private readonly getERC1155TokenURI: AssetsContractController['getERC1155TokenURI']; - - private readonly getNetworkClientById: NetworkController['getNetworkClientById']; - - private readonly onNftAdded?: (data: { - address: string; - symbol: string | undefined; - tokenId: string; - standard: string | null; - source: Source; - }) => void; - - /** - * Creates an NftController instance. - * - * @param options - The controller options. - * @param options.chainId - The chain ID of the current network. - * @param options.onPreferencesStateChange - Allows subscribing to preference controller state changes. - * @param options.onNetworkStateChange - Allows subscribing to network controller state changes. - * @param options.getERC721AssetName - Gets the name of the asset at the given address. - * @param options.getERC721AssetSymbol - Gets the symbol of the asset at the given address. - * @param options.getERC721TokenURI - Gets the URI of the ERC721 token at the given address, with the given ID. - * @param options.getERC721OwnerOf - Get the owner of a ERC-721 NFT. - * @param options.getERC1155BalanceOf - Gets balance of a ERC-1155 NFT. - * @param options.getERC1155TokenURI - Gets the URI of the ERC1155 token at the given address, with the given ID. - * @param options.getNetworkClientById - Gets the network client for the given networkClientId. - * @param options.onNftAdded - Callback that is called when an NFT is added. Currently used pass data - * for tracking the NFT added event. - * @param options.messenger - The controller messenger. - * @param config - Initial options used to configure this controller. - * @param state - Initial state to set on this controller. - */ - constructor( - { - chainId: initialChainId, - onPreferencesStateChange, - onNetworkStateChange, - getERC721AssetName, - getERC721AssetSymbol, - getERC721TokenURI, - getERC721OwnerOf, - getERC1155BalanceOf, - getERC1155TokenURI, - getNetworkClientById, - onNftAdded, - messenger, - }: { - chainId: Hex; - onPreferencesStateChange: ( - listener: (preferencesState: PreferencesState) => void, - ) => void; - onNetworkStateChange: ( - listener: (networkState: NetworkState) => void, - ) => void; - getERC721AssetName: AssetsContractController['getERC721AssetName']; - getERC721AssetSymbol: AssetsContractController['getERC721AssetSymbol']; - getERC721TokenURI: AssetsContractController['getERC721TokenURI']; - getERC721OwnerOf: AssetsContractController['getERC721OwnerOf']; - getERC1155BalanceOf: AssetsContractController['getERC1155BalanceOf']; - getERC1155TokenURI: AssetsContractController['getERC1155TokenURI']; - getNetworkClientById: NetworkController['getNetworkClientById']; - onNftAdded?: (data: { - address: string; - symbol: string | undefined; - tokenId: string; - standard: string | null; - source: string; - }) => void; - messenger: NftControllerMessenger; - }, - config?: Partial, - state?: Partial, - ) { - super(config, state); - this.defaultConfig = { - selectedAddress: '', - chainId: initialChainId, - ipfsGateway: IPFS_DEFAULT_GATEWAY_URL, - openSeaEnabled: false, - useIPFSSubdomains: true, - isIpfsGatewayEnabled: true, - }; - - this.defaultState = getDefaultNftState(); - this.initialize(); - this.getERC721AssetName = getERC721AssetName; - this.getERC721AssetSymbol = getERC721AssetSymbol; - this.getERC721TokenURI = getERC721TokenURI; - this.getERC721OwnerOf = getERC721OwnerOf; - this.getERC1155BalanceOf = getERC1155BalanceOf; - this.getERC1155TokenURI = getERC1155TokenURI; - this.getNetworkClientById = getNetworkClientById; - this.onNftAdded = onNftAdded; - this.messagingSystem = messenger; - - onPreferencesStateChange( - async ({ - selectedAddress, - ipfsGateway, - openSeaEnabled, - isIpfsGatewayEnabled, - }) => { - this.configure({ - selectedAddress, - ipfsGateway, - openSeaEnabled, - isIpfsGatewayEnabled, - }); - - const needsUpdateNftMetadata = - (isIpfsGatewayEnabled && ipfsGateway !== '') || openSeaEnabled; - - if (needsUpdateNftMetadata) { - const { chainId } = this.config; - const nfts: Nft[] = - this.state.allNfts[selectedAddress]?.[chainId] ?? []; - // filter only nfts - const nftsToUpdate = nfts.filter( - (singleNft) => - !singleNft.name && !singleNft.description && !singleNft.image, - ); - if (nftsToUpdate.length !== 0) { - await this.updateNftMetadata({ - nfts: nftsToUpdate, - userAddress: selectedAddress, - }); - } - } - }, - ); - - onNetworkStateChange(({ selectedNetworkClientId }) => { - const selectedNetworkClient = getNetworkClientById( - selectedNetworkClientId, - ); - const { chainId } = selectedNetworkClient.configuration; - - this.configure({ chainId }); - }); - } - - private async validateWatchNft( + async #validateWatchNft( asset: NftAsset, type: NFTStandardType, userAddress: string, @@ -1143,15 +1165,15 @@ export class NftController extends BaseControllerV1 { // temporary method to get the correct chainId until we remove chainId from the config & the chainId arg from the detection logic // Just a helper method to prefer the networkClient chainId first then the chainId argument and then finally the config chainId - private getCorrectChainId({ + #getCorrectChainId({ networkClientId, }: { networkClientId?: NetworkClientId; }) { if (networkClientId) { - return this.getNetworkClientById(networkClientId).configuration.chainId; + return this.#getNetworkClientById(networkClientId).configuration.chainId; } - return this.config.chainId; + return this.#config.chainId; } /** @@ -1174,15 +1196,15 @@ export class NftController extends BaseControllerV1 { origin: string, { networkClientId, - userAddress = this.config.selectedAddress, + userAddress = this.#config.selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string; } = { - userAddress: this.config.selectedAddress, + userAddress: this.#config.selectedAddress, }, ) { - await this.validateWatchNft(asset, type, userAddress); + await this.#validateWatchNft(asset, type, userAddress); const nftMetadata = await this.getNftInformation( asset.address, @@ -1252,7 +1274,7 @@ export class NftController extends BaseControllerV1 { ): Promise { // Checks the ownership for ERC-721. try { - const owner = await this.getERC721OwnerOf( + const owner = await this.#getERC721OwnerOf( nftAddress, tokenId, networkClientId, @@ -1265,7 +1287,7 @@ export class NftController extends BaseControllerV1 { // Checks the ownership for ERC-1155. try { - const balance = await this.getERC1155BalanceOf( + const balance = await this.#getERC1155BalanceOf( ownerAddress, nftAddress, tokenId, @@ -1297,7 +1319,7 @@ export class NftController extends BaseControllerV1 { address: string, tokenId: string, { - userAddress = this.config.selectedAddress, + userAddress = this.#config.selectedAddress, networkClientId, source, }: { @@ -1305,7 +1327,7 @@ export class NftController extends BaseControllerV1 { networkClientId?: NetworkClientId; source?: Source; } = { - userAddress: this.config.selectedAddress, + userAddress: this.#config.selectedAddress, }, ) { if ( @@ -1339,7 +1361,7 @@ export class NftController extends BaseControllerV1 { tokenId: string, { nftMetadata, - userAddress = this.config.selectedAddress, + userAddress = this.#config.selectedAddress, source = Source.Custom, networkClientId, }: { @@ -1347,18 +1369,22 @@ export class NftController extends BaseControllerV1 { userAddress?: string; source?: Source; networkClientId?: NetworkClientId; - } = { userAddress: this.config.selectedAddress }, + } = { userAddress: this.#config.selectedAddress }, ) { - tokenAddress = toChecksumHexAddress(tokenAddress); + const checksumHexAddress = toChecksumHexAddress(tokenAddress); - const chainId = this.getCorrectChainId({ networkClientId }); + const chainId = this.#getCorrectChainId({ networkClientId }); nftMetadata = nftMetadata || - (await this.getNftInformation(tokenAddress, tokenId, networkClientId)); + (await this.getNftInformation( + checksumHexAddress, + tokenId, + networkClientId, + )); - const newNftContracts = await this.addNftContract({ - tokenAddress, + const newNftContracts = await this.#addNftContract({ + tokenAddress: checksumHexAddress, userAddress, networkClientId, source, @@ -1368,13 +1394,13 @@ export class NftController extends BaseControllerV1 { // If NFT contract was not added, do not add individual NFT const nftContract = newNftContracts.find( (contract) => - contract.address.toLowerCase() === tokenAddress.toLowerCase(), + contract.address.toLowerCase() === checksumHexAddress.toLowerCase(), ); // If NFT contract information, add individual NFT if (nftContract) { - await this.addIndividualNft( - tokenAddress, + await this.#addIndividualNft( + checksumHexAddress, tokenId, nftMetadata, nftContract, @@ -1395,14 +1421,14 @@ export class NftController extends BaseControllerV1 { */ async updateNftMetadata({ nfts, - userAddress = this.config.selectedAddress, + userAddress = this.#config.selectedAddress, networkClientId, }: { nfts: Nft[]; userAddress?: string; networkClientId?: NetworkClientId; }) { - const chainId = this.getCorrectChainId({ networkClientId }); + const chainId = this.#getCorrectChainId({ networkClientId }); const nftsWithChecksumAdr = nfts.map((nft) => { return { @@ -1478,22 +1504,25 @@ export class NftController extends BaseControllerV1 { tokenId: string, { networkClientId, - userAddress = this.config.selectedAddress, + userAddress = this.#config.selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string } = { - userAddress: this.config.selectedAddress, + userAddress: this.#config.selectedAddress, }, ) { - const chainId = this.getCorrectChainId({ networkClientId }); - address = toChecksumHexAddress(address); - this.removeIndividualNft(address, tokenId, { chainId, userAddress }); + const chainId = this.#getCorrectChainId({ networkClientId }); + const checksumHexAddress = toChecksumHexAddress(address); + this.#removeIndividualNft(checksumHexAddress, tokenId, { + chainId, + userAddress, + }); const { allNfts } = this.state; const nfts = allNfts[userAddress]?.[chainId] || []; const remainingNft = nfts.find( - (nft) => nft.address.toLowerCase() === address.toLowerCase(), + (nft) => nft.address.toLowerCase() === checksumHexAddress.toLowerCase(), ); if (!remainingNft) { - this.removeNftContract(address, { chainId, userAddress }); + this.#removeNftContract(checksumHexAddress, { chainId, userAddress }); } } @@ -1511,24 +1540,24 @@ export class NftController extends BaseControllerV1 { tokenId: string, { networkClientId, - userAddress = this.config.selectedAddress, + userAddress = this.#config.selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string } = { - userAddress: this.config.selectedAddress, + userAddress: this.#config.selectedAddress, }, ) { - const chainId = this.getCorrectChainId({ networkClientId }); - address = toChecksumHexAddress(address); - this.removeAndIgnoreIndividualNft(address, tokenId, { + const chainId = this.#getCorrectChainId({ networkClientId }); + const checksumHexAddress = toChecksumHexAddress(address); + this.#removeAndIgnoreIndividualNft(checksumHexAddress, tokenId, { chainId, userAddress, }); const { allNfts } = this.state; const nfts = allNfts[userAddress]?.[chainId] || []; const remainingNft = nfts.find( - (nft) => nft.address.toLowerCase() === address.toLowerCase(), + (nft) => nft.address.toLowerCase() === checksumHexAddress.toLowerCase(), ); if (!remainingNft) { - this.removeNftContract(address, { chainId, userAddress }); + this.#removeNftContract(checksumHexAddress, { chainId, userAddress }); } } @@ -1536,7 +1565,9 @@ export class NftController extends BaseControllerV1 { * Removes all NFTs from the ignored list. */ clearIgnoredNfts() { - this.update({ ignoredNfts: [] }); + this.update((state) => { + state.ignoredNfts = []; + }); } /** @@ -1554,13 +1585,13 @@ export class NftController extends BaseControllerV1 { nft: Nft, batch: boolean, { - userAddress = this.config.selectedAddress, + userAddress = this.#config.selectedAddress, networkClientId, }: { networkClientId?: NetworkClientId; userAddress?: string } = { - userAddress: this.config.selectedAddress, + userAddress: this.#config.selectedAddress, }, ) { - const chainId = this.getCorrectChainId({ networkClientId }); + const chainId = this.#getCorrectChainId({ networkClientId }); const { address, tokenId } = nft; let isOwned = nft.isCurrentlyOwned; try { @@ -1573,28 +1604,36 @@ export class NftController extends BaseControllerV1 { // we want to keep the current value of isCurrentlyOwned for this flow. } - nft.isCurrentlyOwned = isOwned; + const updatedNft = { + ...nft, + isCurrentlyOwned: isOwned, + }; if (batch) { - return nft; + return updatedNft; } // if this is not part of a batched update we update this one NFT in state const { allNfts } = this.state; - const nfts = allNfts[userAddress]?.[chainId] || []; - const nftToUpdate = nfts.find( + const nfts = [...(allNfts[userAddress]?.[chainId] || [])]; + const indexToUpdate = nfts.findIndex( (item) => item.tokenId === tokenId && item.address.toLowerCase() === address.toLowerCase(), ); - if (nftToUpdate) { - nftToUpdate.isCurrentlyOwned = isOwned; - this.updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, { + + if (indexToUpdate !== -1) { + nfts[indexToUpdate] = updatedNft; + this.update((state) => { + state.allNfts[userAddress][chainId] = nfts; + }); + this.#updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, { userAddress, chainId, }); } - return nft; + + return updatedNft; } /** @@ -1607,12 +1646,12 @@ export class NftController extends BaseControllerV1 { async checkAndUpdateAllNftsOwnershipStatus( { networkClientId, - userAddress = this.config.selectedAddress, + userAddress = this.#config.selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string } = { - userAddress: this.config.selectedAddress, + userAddress: this.#config.selectedAddress, }, ) { - const chainId = this.getCorrectChainId({ networkClientId }); + const chainId = this.#getCorrectChainId({ networkClientId }); const { allNfts } = this.state; const nfts = allNfts[userAddress]?.[chainId] || []; const updatedNfts = await Promise.all( @@ -1626,7 +1665,7 @@ export class NftController extends BaseControllerV1 { }), ); - this.updateNestedNftState(updatedNfts, ALL_NFTS_STATE_KEY, { + this.#updateNestedNftState(updatedNfts, ALL_NFTS_STATE_KEY, { userAddress, chainId, }); @@ -1648,17 +1687,17 @@ export class NftController extends BaseControllerV1 { favorite: boolean, { networkClientId, - userAddress = this.config.selectedAddress, + userAddress = this.#config.selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string; } = { - userAddress: this.config.selectedAddress, + userAddress: this.#config.selectedAddress, }, ) { - const chainId = this.getCorrectChainId({ networkClientId }); + const chainId = this.#getCorrectChainId({ networkClientId }); const { allNfts } = this.state; - const nfts = allNfts[userAddress]?.[chainId] || []; + const nfts = [...(allNfts[userAddress]?.[chainId] || [])]; const index: number = nfts.findIndex( (nft) => nft.address === address && nft.tokenId === tokenId, ); @@ -1675,7 +1714,7 @@ export class NftController extends BaseControllerV1 { // Update Nfts array nfts[index] = updatedNft; - this.updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, { + this.#updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, { chainId, userAddress, }); @@ -1748,7 +1787,7 @@ export class NftController extends BaseControllerV1 { updatedNft, ...nfts.slice(nftInfo.index + 1), ]; - this.updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { + this.#updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { chainId, userAddress: selectedAddress, }); @@ -1787,7 +1826,7 @@ export class NftController extends BaseControllerV1 { ...nfts.slice(index + 1), ]; - this.updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { + this.#updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, { chainId, userAddress: selectedAddress, }); diff --git a/packages/assets-controllers/src/NftDetectionController.test.ts b/packages/assets-controllers/src/NftDetectionController.test.ts index 9f06ee94215..8801439f7c2 100644 --- a/packages/assets-controllers/src/NftDetectionController.test.ts +++ b/packages/assets-controllers/src/NftDetectionController.test.ts @@ -12,7 +12,10 @@ import { FakeBlockTracker } from '../../../tests/fake-block-tracker'; import { FakeProvider } from '../../../tests/fake-provider'; import { advanceTime } from '../../../tests/helpers'; import { Source } from './constants'; -import { getDefaultNftState, type NftState } from './NftController'; +import { + defaultNftControllerState, + type NftControllerState, +} from './NftController'; import { type NftDetectionConfig, NftDetectionController, @@ -575,7 +578,7 @@ describe('NftDetectionController', () => { const mockAddNft = jest.fn(); const mockGetNftState = jest.fn().mockImplementation(() => { return { - ...getDefaultNftState(), + ...defaultNftControllerState, ignoredNfts: [ // This address and token ID are always detected, as determined by // the nock mocks setup in `beforeEach` @@ -846,7 +849,7 @@ type WithControllerCallback = ({ controller, }: { controller: NftDetectionController; - triggerNftStateChange: (state: NftState) => void; + triggerNftStateChange: (state: NftControllerState) => void; triggerPreferencesStateChange: (state: PreferencesState) => void; }) => Promise | ReturnValue; @@ -885,7 +888,7 @@ async function withController( }; }); - const nftStateChangeListeners: ((state: NftState) => void)[] = []; + const nftStateChangeListeners: ((state: NftControllerState) => void)[] = []; const preferencesStateChangeListeners: ((state: PreferencesState) => void)[] = []; const controller = new NftDetectionController( @@ -902,7 +905,7 @@ async function withController( addNft: jest.fn(), getNftApi: jest.fn(), getNetworkClientById, - getNftState: getDefaultNftState, + getNftState: getDefaultNftControllerState, disabled: true, selectedAddress: '', ...options, @@ -912,7 +915,7 @@ async function withController( try { return await fn({ controller, - triggerNftStateChange: (state: NftState) => { + triggerNftStateChange: (state: NftControllerState) => { for (const listener of nftStateChangeListeners) { listener(state); } diff --git a/packages/assets-controllers/src/NftDetectionController.ts b/packages/assets-controllers/src/NftDetectionController.ts index 488d468d861..d63eb3121c4 100644 --- a/packages/assets-controllers/src/NftDetectionController.ts +++ b/packages/assets-controllers/src/NftDetectionController.ts @@ -18,7 +18,7 @@ import type { Hex } from '@metamask/utils'; import { Source } from './constants'; import { type NftController, - type NftState, + type NftControllerState, type NftMetadata, } from './NftController'; @@ -413,7 +413,7 @@ export class NftDetectionController extends StaticIntervalPollingControllerV1< private readonly getNftApi: NftController['getNftApi']; - private readonly getNftState: () => NftState; + private readonly getNftState: () => NftControllerState; private readonly getNetworkClientById: NetworkController['getNetworkClientById']; @@ -450,7 +450,9 @@ export class NftDetectionController extends StaticIntervalPollingControllerV1< }: { chainId: Hex; getNetworkClientById: NetworkController['getNetworkClientById']; - onNftsStateChange: (listener: (nftsState: NftState) => void) => void; + onNftsStateChange: ( + listener: (nftsState: NftControllerState) => void, + ) => void; onPreferencesStateChange: ( listener: (preferencesState: PreferencesState) => void, ) => void; @@ -460,7 +462,7 @@ export class NftDetectionController extends StaticIntervalPollingControllerV1< getOpenSeaApiKey: () => string | undefined; addNft: NftController['addNft']; getNftApi: NftController['getNftApi']; - getNftState: () => NftState; + getNftState: () => NftControllerState; disabled: boolean; selectedAddress: string; }, From 3245f449832a6cc8f75eb6a5563a787e83fe4695 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 23 May 2024 13:43:54 +0200 Subject: [PATCH 02/17] fix: token detection unit tests --- packages/assets-controllers/src/NftDetectionController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/NftDetectionController.test.ts b/packages/assets-controllers/src/NftDetectionController.test.ts index 8801439f7c2..7a2da323add 100644 --- a/packages/assets-controllers/src/NftDetectionController.test.ts +++ b/packages/assets-controllers/src/NftDetectionController.test.ts @@ -905,7 +905,7 @@ async function withController( addNft: jest.fn(), getNftApi: jest.fn(), getNetworkClientById, - getNftState: getDefaultNftControllerState, + getNftState: () => defaultNftControllerState, disabled: true, selectedAddress: '', ...options, From b4b070a8d43d3727f4c8ee1d243e702bcb95b598 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 23 May 2024 14:26:54 +0200 Subject: [PATCH 03/17] fix: Prototype-polluting assignment --- packages/assets-controllers/src/NftController.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index 0c3d947a7f7..c8004ae4a89 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -1625,7 +1625,13 @@ export class NftController extends BaseController< if (indexToUpdate !== -1) { nfts[indexToUpdate] = updatedNft; this.update((state) => { - state.allNfts[userAddress][chainId] = nfts; + state.allNfts[userAddress] = Object.assign( + {}, + state.allNfts[userAddress], + { + [chainId]: nfts, + }, + ); }); this.#updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, { userAddress, From 04e876d10ed48314d9433ca431441d66a116967c Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Mon, 27 May 2024 15:12:12 +0200 Subject: [PATCH 04/17] fix: remove controller options extra type --- .../assets-controllers/src/NftController.ts | 95 +++++++++---------- 1 file changed, 46 insertions(+), 49 deletions(-) diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index c8004ae4a89..1673716b6bb 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -193,53 +193,6 @@ export type NftControllerState = { ignoredNfts: Nft[]; }; -/** - * Creates an NftController instance. - * - * @param options - The controller options. - * @param options.chainId - The chain ID of the current network. - * @param options.onPreferencesStateChange - Allows subscribing to preference controller state changes. - * @param options.onNetworkStateChange - Allows subscribing to network controller state changes. - * @param options.getERC721AssetName - Gets the name of the asset at the given address. - * @param options.getERC721AssetSymbol - Gets the symbol of the asset at the given address. - * @param options.getERC721TokenURI - Gets the URI of the ERC721 token at the given address, with the given ID. - * @param options.getERC721OwnerOf - Get the owner of a ERC-721 NFT. - * @param options.getERC1155BalanceOf - Gets balance of a ERC-1155 NFT. - * @param options.getERC1155TokenURI - Gets the URI of the ERC1155 token at the given address, with the given ID. - * @param options.getNetworkClientById - Gets the network client for the given networkClientId. - * @param options.onNftAdded - Callback that is called when an NFT is added. Currently used pass data - * for tracking the NFT added event. - * @param options.messenger - The controller messenger. - * @param options.config - Initial options used to configure this controller. - * @param options.state - Initial state to set on this controller. - */ -export type NftConftrollerOptions = { - chainId: Hex; - onPreferencesStateChange: ( - listener: (preferencesState: PreferencesState) => void, - ) => void; - onNetworkStateChange: ( - listener: (networkState: NetworkState) => void, - ) => void; - getERC721AssetName: AssetsContractController['getERC721AssetName']; - getERC721AssetSymbol: AssetsContractController['getERC721AssetSymbol']; - getERC721TokenURI: AssetsContractController['getERC721TokenURI']; - getERC721OwnerOf: AssetsContractController['getERC721OwnerOf']; - getERC1155BalanceOf: AssetsContractController['getERC1155BalanceOf']; - getERC1155TokenURI: AssetsContractController['getERC1155TokenURI']; - getNetworkClientById: NetworkController['getNetworkClientById']; - onNftAdded?: (data: { - address: string; - symbol: string | undefined; - tokenId: string; - standard: string | null; - source: string; - }) => void; - messenger: NftControllerMessenger; - config?: Partial; - state?: Partial; -}; - const metadata = { allNftContracts: { persist: true, anonymous: false }, allNfts: { persist: true, anonymous: false }, @@ -329,6 +282,26 @@ export class NftController extends BaseController< source: Source; }) => void; + /** + * Creates an NftController instance. + * + * @param options - The controller options. + * @param options.chainId - The chain ID of the current network. + * @param options.onPreferencesStateChange - Allows subscribing to preference controller state changes. + * @param options.onNetworkStateChange - Allows subscribing to network controller state changes. + * @param options.getERC721AssetName - Gets the name of the asset at the given address. + * @param options.getERC721AssetSymbol - Gets the symbol of the asset at the given address. + * @param options.getERC721TokenURI - Gets the URI of the ERC721 token at the given address, with the given ID. + * @param options.getERC721OwnerOf - Get the owner of a ERC-721 NFT. + * @param options.getERC1155BalanceOf - Gets balance of a ERC-1155 NFT. + * @param options.getERC1155TokenURI - Gets the URI of the ERC1155 token at the given address, with the given ID. + * @param options.getNetworkClientById - Gets the network client for the given networkClientId. + * @param options.onNftAdded - Callback that is called when an NFT is added. Currently used pass data + * for tracking the NFT added event. + * @param options.messenger - The controller messenger. + * @param options.config - Initial options used to configure this controller. + * @param options.state - Initial state to set on this controller. + */ constructor({ chainId: initialChainId, onPreferencesStateChange, @@ -344,7 +317,32 @@ export class NftController extends BaseController< messenger, config = {}, state = {}, - }: NftConftrollerOptions) { + }: { + chainId: Hex; + onPreferencesStateChange: ( + listener: (preferencesState: PreferencesState) => void, + ) => void; + onNetworkStateChange: ( + listener: (networkState: NetworkState) => void, + ) => void; + getERC721AssetName: AssetsContractController['getERC721AssetName']; + getERC721AssetSymbol: AssetsContractController['getERC721AssetSymbol']; + getERC721TokenURI: AssetsContractController['getERC721TokenURI']; + getERC721OwnerOf: AssetsContractController['getERC721OwnerOf']; + getERC1155BalanceOf: AssetsContractController['getERC1155BalanceOf']; + getERC1155TokenURI: AssetsContractController['getERC1155TokenURI']; + getNetworkClientById: NetworkController['getNetworkClientById']; + onNftAdded?: (data: { + address: string; + symbol: string | undefined; + tokenId: string; + standard: string | null; + source: string; + }) => void; + messenger: NftControllerMessenger; + config?: Partial; + state?: Partial; + }) { super({ name: controllerName, metadata, @@ -372,7 +370,6 @@ export class NftController extends BaseController< this.#getERC1155TokenURI = getERC1155TokenURI; this.#getNetworkClientById = getNetworkClientById; this.#onNftAdded = onNftAdded; - this.messagingSystem = messenger; onPreferencesStateChange( async ({ From 8a96da5f8ec3391253546e10c92a6f0eeccc37f6 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Mon, 27 May 2024 17:24:20 +0200 Subject: [PATCH 05/17] fix: use messenger instead of constructor callback --- .../src/NftController.test.ts | 53 +++---- .../assets-controllers/src/NftController.ts | 145 ++++++++++-------- 2 files changed, 108 insertions(+), 90 deletions(-) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 71434698e85..dc32785a59e 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -21,11 +21,13 @@ import { import type { NetworkClientConfiguration, NetworkClientId, + NetworkControllerNetworkDidChangeEvent, NetworkState, } from '@metamask/network-controller'; import { defaultState as defaultNetworkState } from '@metamask/network-controller'; import { getDefaultPreferencesState, + type PreferencesControllerStateChangeEvent, type PreferencesState, } from '@metamask/preferences-controller'; import BN from 'bn.js'; @@ -132,23 +134,12 @@ function setupController({ NetworkClientConfiguration >; } = {}) { - const onNetworkDidChangeListeners: ((state: NetworkState) => void)[] = []; - const changeNetwork = ({ - selectedNetworkClientId, - }: { - selectedNetworkClientId: NetworkClientId; - }) => { - onNetworkDidChangeListeners.forEach((listener) => { - listener({ - ...defaultNetworkState, - selectedNetworkClientId, - }); - }); - }; - const messenger = new ControllerMessenger< ExtractAvailableAction | GetApprovalsState, - ExtractAvailableEvent | ApprovalStateChange + | ExtractAvailableEvent + | ApprovalStateChange + | PreferencesControllerStateChangeEvent + | NetworkControllerNetworkDidChangeEvent >(); const getNetworkClientById = buildMockGetNetworkClientById( @@ -176,34 +167,40 @@ function setupController({ 'ApprovalController:addRequest', 'NetworkController:getNetworkClientById', ], - allowedEvents: [], + allowedEvents: [ + 'NetworkController:networkDidChange', + 'PreferencesController:stateChange', + ], }); - const preferencesStateChangeListeners: ((state: PreferencesState) => void)[] = - []; const nftController = new NftController({ chainId: ChainId.mainnet, - onPreferencesStateChange: (listener) => { - preferencesStateChangeListeners.push(listener); - }, - onNetworkStateChange: (listener) => - onNetworkDidChangeListeners.push(listener), getERC721AssetName: jest.fn(), getERC721AssetSymbol: jest.fn(), getERC721TokenURI: jest.fn(), getERC721OwnerOf: jest.fn(), getERC1155BalanceOf: jest.fn(), getERC1155TokenURI: jest.fn(), - getNetworkClientById, onNftAdded: jest.fn(), messenger: nftControllerMessenger, ...options, }); + const triggerPreferencesStateChange = (state: PreferencesState) => { - for (const listener of preferencesStateChangeListeners) { - listener(state); - } + messenger.publish('PreferencesController:stateChange', state, []); + }; + + const changeNetwork = ({ + selectedNetworkClientId, + }: { + selectedNetworkClientId: NetworkClientId; + }) => { + messenger.publish('NetworkController:networkDidChange', { + ...defaultNetworkState, + selectedNetworkClientId, + }); }; + triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, @@ -212,9 +209,9 @@ function setupController({ return { nftController, - changeNetwork, messenger, approvalController, + changeNetwork, triggerPreferencesStateChange, }; } diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index 1673716b6bb..94b2e4ff5d5 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -19,11 +19,14 @@ import { } from '@metamask/controller-utils'; import type { NetworkClientId, - NetworkController, NetworkControllerGetNetworkClientByIdAction, + NetworkControllerNetworkDidChangeEvent, NetworkState, } from '@metamask/network-controller'; -import type { PreferencesState } from '@metamask/preferences-controller'; +import type { + PreferencesControllerStateChangeEvent, + PreferencesState, +} from '@metamask/preferences-controller'; import { rpcErrors } from '@metamask/rpc-errors'; import type { Hex } from '@metamask/utils'; import { remove0x } from '@metamask/utils'; @@ -215,10 +218,14 @@ const controllerName = 'NftController'; /** * The external actions available to the {@link NftController}. */ -type AllowedActions = +export type AllowedActions = | AddApprovalRequest | NetworkControllerGetNetworkClientByIdAction; +export type AllowEdEvents = + | PreferencesControllerStateChangeEvent + | NetworkControllerNetworkDidChangeEvent; + export type NftControllerStateChangeEvent = ControllerStateChangeEvent< typeof controllerName, NftControllerState @@ -232,9 +239,9 @@ export type NFtControllerEvents = NftControllerStateChangeEvent; export type NftControllerMessenger = RestrictedControllerMessenger< typeof controllerName, AllowedActions, - NFtControllerEvents, + NFtControllerEvents | AllowEdEvents, AllowedActions['type'], - never + AllowEdEvents['type'] >; export const defaultNftControllerState: NftControllerState = { @@ -272,8 +279,6 @@ export class NftController extends BaseController< readonly #getERC1155TokenURI: AssetsContractController['getERC1155TokenURI']; - readonly #getNetworkClientById: NetworkController['getNetworkClientById']; - readonly #onNftAdded?: (data: { address: string; symbol: string | undefined; @@ -287,15 +292,12 @@ export class NftController extends BaseController< * * @param options - The controller options. * @param options.chainId - The chain ID of the current network. - * @param options.onPreferencesStateChange - Allows subscribing to preference controller state changes. - * @param options.onNetworkStateChange - Allows subscribing to network controller state changes. * @param options.getERC721AssetName - Gets the name of the asset at the given address. * @param options.getERC721AssetSymbol - Gets the symbol of the asset at the given address. * @param options.getERC721TokenURI - Gets the URI of the ERC721 token at the given address, with the given ID. * @param options.getERC721OwnerOf - Get the owner of a ERC-721 NFT. * @param options.getERC1155BalanceOf - Gets balance of a ERC-1155 NFT. * @param options.getERC1155TokenURI - Gets the URI of the ERC1155 token at the given address, with the given ID. - * @param options.getNetworkClientById - Gets the network client for the given networkClientId. * @param options.onNftAdded - Callback that is called when an NFT is added. Currently used pass data * for tracking the NFT added event. * @param options.messenger - The controller messenger. @@ -304,34 +306,24 @@ export class NftController extends BaseController< */ constructor({ chainId: initialChainId, - onPreferencesStateChange, - onNetworkStateChange, getERC721AssetName, getERC721AssetSymbol, getERC721TokenURI, getERC721OwnerOf, getERC1155BalanceOf, getERC1155TokenURI, - getNetworkClientById, onNftAdded, messenger, config = {}, state = {}, }: { chainId: Hex; - onPreferencesStateChange: ( - listener: (preferencesState: PreferencesState) => void, - ) => void; - onNetworkStateChange: ( - listener: (networkState: NetworkState) => void, - ) => void; getERC721AssetName: AssetsContractController['getERC721AssetName']; getERC721AssetSymbol: AssetsContractController['getERC721AssetSymbol']; getERC721TokenURI: AssetsContractController['getERC721TokenURI']; getERC721OwnerOf: AssetsContractController['getERC721OwnerOf']; getERC1155BalanceOf: AssetsContractController['getERC1155BalanceOf']; getERC1155TokenURI: AssetsContractController['getERC1155TokenURI']; - getNetworkClientById: NetworkController['getNetworkClientById']; onNftAdded?: (data: { address: string; symbol: string | undefined; @@ -368,53 +360,76 @@ export class NftController extends BaseController< this.#getERC721OwnerOf = getERC721OwnerOf; this.#getERC1155BalanceOf = getERC1155BalanceOf; this.#getERC1155TokenURI = getERC1155TokenURI; - this.#getNetworkClientById = getNetworkClientById; this.#onNftAdded = onNftAdded; - onPreferencesStateChange( - async ({ - selectedAddress, - ipfsGateway, - openSeaEnabled, - isIpfsGatewayEnabled, - }) => { - this.#config = { - ...this.#config, - selectedAddress, - ipfsGateway, - openSeaEnabled, - isIpfsGatewayEnabled, - }; + this.messagingSystem.subscribe( + 'PreferencesController:stateChange', + this.#onPreferencesControllerStateChange.bind(this), + ); - const needsUpdateNftMetadata = - (isIpfsGatewayEnabled && ipfsGateway !== '') || openSeaEnabled; - - if (needsUpdateNftMetadata) { - const { chainId } = this.#config; - const nfts: Nft[] = - this.state.allNfts[selectedAddress]?.[chainId] ?? []; - // filter only nfts - const nftsToUpdate = nfts.filter( - (singleNft) => - !singleNft.name && !singleNft.description && !singleNft.image, - ); - if (nftsToUpdate.length !== 0) { - await this.updateNftMetadata({ - nfts: nftsToUpdate, - userAddress: selectedAddress, - }); - } - } - }, + this.messagingSystem.subscribe( + 'NetworkController:networkDidChange', + this.#onNetworkControllerNetworkDidChange.bind(this), ); + } - onNetworkStateChange(({ selectedNetworkClientId }) => { - const selectedNetworkClient = getNetworkClientById( - selectedNetworkClientId, + /** + * Handles the network change on the network controller. + * @param networkState - The new state of the preference controller. + * @param networkState.selectedNetworkClientId - The current selected network client id. + */ + #onNetworkControllerNetworkDidChange({ + selectedNetworkClientId, + }: NetworkState) { + const { + configuration: { chainId }, + } = this.messagingSystem.call( + 'NetworkController:getNetworkClientById', + selectedNetworkClientId, + ); + this.#config.chainId = chainId; + } + + /** + * Handles the state change of the preference controller. + * @param preferencesState - The new state of the preference controller. + * @param preferencesState.selectedAddress - The current selected address. + * @param preferencesState.ipfsGateway - The configured IPFS gateway. + * @param preferencesState.openSeaEnabled - Controls whether the OpenSea API is used. + * @param preferencesState.isIpfsGatewayEnabled - Controls whether IPFS is enabled or not. + */ + async #onPreferencesControllerStateChange({ + selectedAddress, + ipfsGateway, + openSeaEnabled, + isIpfsGatewayEnabled, + }: PreferencesState) { + this.#config = { + ...this.#config, + selectedAddress, + ipfsGateway, + openSeaEnabled, + isIpfsGatewayEnabled, + }; + + const needsUpdateNftMetadata = + (isIpfsGatewayEnabled && ipfsGateway !== '') || openSeaEnabled; + + if (needsUpdateNftMetadata) { + const { chainId } = this.#config; + const nfts: Nft[] = this.state.allNfts[selectedAddress]?.[chainId] ?? []; + // filter only nfts + const nftsToUpdate = nfts.filter( + (singleNft) => + !singleNft.name && !singleNft.description && !singleNft.image, ); - const { chainId } = selectedNetworkClient.configuration; - this.#config.chainId = chainId; - }); + if (nftsToUpdate.length !== 0) { + await this.updateNftMetadata({ + nfts: nftsToUpdate, + userAddress: selectedAddress, + }); + } + } } getNftApi() { @@ -1168,7 +1183,13 @@ export class NftController extends BaseController< networkClientId?: NetworkClientId; }) { if (networkClientId) { - return this.#getNetworkClientById(networkClientId).configuration.chainId; + const { + configuration: { chainId }, + } = this.messagingSystem.call( + 'NetworkController:getNetworkClientById', + networkClientId, + ); + return chainId; } return this.#config.chainId; } From 980039de45c8a6171d979a4447f33d6a235542a4 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Tue, 28 May 2024 09:46:21 +0200 Subject: [PATCH 06/17] fix: remove unused import --- 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 dc32785a59e..98ed77a4ae7 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -22,7 +22,6 @@ import type { NetworkClientConfiguration, NetworkClientId, NetworkControllerNetworkDidChangeEvent, - NetworkState, } from '@metamask/network-controller'; import { defaultState as defaultNetworkState } from '@metamask/network-controller'; import { From 62f28ebb7f3ed1e9539f5bf00ab17acf5d965e50 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Tue, 28 May 2024 15:24:44 +0200 Subject: [PATCH 07/17] fix: typos --- .../assets-controllers/src/NftController.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index 94b2e4ff5d5..06b6cbe2a52 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -170,7 +170,7 @@ export type NftConfig = { chainId: Hex; ipfsGateway: string; openSeaEnabled: boolean; - useIPFSSubdomains: boolean; + useIpfsSubdomains: boolean; isIpfsGatewayEnabled: boolean; }; @@ -222,7 +222,7 @@ export type AllowedActions = | AddApprovalRequest | NetworkControllerGetNetworkClientByIdAction; -export type AllowEdEvents = +export type AllowedEvents = | PreferencesControllerStateChangeEvent | NetworkControllerNetworkDidChangeEvent; @@ -231,7 +231,7 @@ export type NftControllerStateChangeEvent = ControllerStateChangeEvent< NftControllerState >; -export type NFtControllerEvents = NftControllerStateChangeEvent; +export type NftControllerEvents = NftControllerStateChangeEvent; /** * The messenger of the {@link NftController}. @@ -239,9 +239,9 @@ export type NFtControllerEvents = NftControllerStateChangeEvent; export type NftControllerMessenger = RestrictedControllerMessenger< typeof controllerName, AllowedActions, - NFtControllerEvents | AllowEdEvents, + NftControllerEvents | AllowedEvents, AllowedActions['type'], - AllowEdEvents['type'] + AllowedEvents['type'] >; export const defaultNftControllerState: NftControllerState = { @@ -349,7 +349,7 @@ export class NftController extends BaseController< chainId: initialChainId, ipfsGateway: IPFS_DEFAULT_GATEWAY_URL, openSeaEnabled: false, - useIPFSSubdomains: true, + useIpfsSubdomains: true, isIpfsGatewayEnabled: true, ...config, }; @@ -570,7 +570,7 @@ export class NftController extends BaseController< tokenId: string, networkClientId?: NetworkClientId, ): Promise { - const { ipfsGateway, useIPFSSubdomains, isIpfsGatewayEnabled } = + const { ipfsGateway, useIpfsSubdomains, isIpfsGatewayEnabled } = this.#config; const result = await this.getNftURIAndStandard( contractAddress, @@ -606,7 +606,7 @@ export class NftController extends BaseController< } if (hasIpfsTokenURI) { - tokenURI = getFormattedIpfsUrl(ipfsGateway, tokenURI, useIPFSSubdomains); + tokenURI = getFormattedIpfsUrl(ipfsGateway, tokenURI, useIpfsSubdomains); } try { @@ -844,7 +844,7 @@ export class NftController extends BaseController< const checksumHexAddress = toChecksumHexAddress(tokenAddress); const { allNfts } = this.state; - const nfts = [...(allNfts[userAddress]?.[chainId] || [])]; + const nfts = [...(allNfts[userAddress]?.[chainId] ?? [])]; const existingEntry = nfts.find( (nft) => From dd85262f7b769db022502b89c104d067cec66b5b Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Tue, 28 May 2024 23:14:41 +0200 Subject: [PATCH 08/17] fix: unit tests --- .../src/NftController.test.ts | 631 +++++++----------- .../assets-controllers/src/NftController.ts | 45 +- 2 files changed, 272 insertions(+), 404 deletions(-) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 98ed77a4ae7..0ee96888e23 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -1141,7 +1141,7 @@ describe('NftController', () => { name: 'name', image: 'image', description: 'description', - standard: 'ERC721', + standard: ERC721, favorite: false, }, userAddress: detectedUserAddress, @@ -1152,22 +1152,30 @@ describe('NftController', () => { source: 'detected', tokenId: '2', address: '0x01', - standard: 'ERC721', + standard: ERC721, }); }); it('should add NFT by selected address', async () => { - const { nftController, triggerPreferencesStateChange } = - setupController(); + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const mockGetERC1155TokenURI = jest.fn().mockRejectedValue(''); + + const { nftController, triggerPreferencesStateChange } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + getERC1155TokenURI: mockGetERC1155TokenURI, + }, + }); const { chainId } = nftController.getConfig(); const firstAddress = '0x123'; const secondAddress = '0x321'; - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + nock('https://url').get('/').reply(200, { + name: 'name', + image: 'url', + description: 'description', + }); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, @@ -1192,6 +1200,8 @@ describe('NftController', () => { description: 'description', image: 'url', name: 'name', + standard: ERC721, + tokenURI, tokenId: '1234', favorite: false, isCurrentlyOwned: true, @@ -1361,7 +1371,7 @@ describe('NftController', () => { name: 'Kudos Name (directly from tokenURI)', description: 'Kudos Description (directly from tokenURI)', tokenId: ERC721_KUDOS_TOKEN_ID, - standard: 'ERC721', + standard: ERC721, favorite: false, isCurrentlyOwned: true, tokenURI: @@ -1374,7 +1384,7 @@ describe('NftController', () => { address: ERC721_KUDOSADDRESS, name: 'KudosToken', symbol: 'KDO', - schemaName: 'ERC721', + schemaName: ERC721, }); }); @@ -1444,11 +1454,11 @@ describe('NftController', () => { description: 'Kudos Description (directly from tokenURI)', }); const { selectedAddress, chainId } = nftController.getConfig(); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformationFromApi' as any) - .returns(undefined); + nock('https://nft.api.cx.metamask.io') + .get( + '/tokens?chainIds=1&tokens=0x2aEa4Add166EBf38b63d09a75dE1a7b94Aa24163%3A1203&includeTopBid=true&includeAttributes=true&includeLastSale=true', + ) + .reply(404, { error: 'Not found' }); await nftController.addNft(ERC721_KUDOSADDRESS, ERC721_KUDOS_TOKEN_ID); @@ -1460,7 +1470,7 @@ describe('NftController', () => { name: 'Kudos Name (directly from tokenURI)', description: 'Kudos Description (directly from tokenURI)', tokenId: ERC721_KUDOS_TOKEN_ID, - standard: 'ERC721', + standard: ERC721, favorite: false, isCurrentlyOwned: true, tokenURI: @@ -1473,18 +1483,24 @@ describe('NftController', () => { address: ERC721_KUDOSADDRESS, name: 'KudosToken', symbol: 'KDO', - schemaName: 'ERC721', + schemaName: ERC721, }); }); it('should add NFT by provider type', async () => { - const { nftController, changeNetwork } = setupController(); + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController, changeNetwork } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); const { selectedAddress } = nftController.getConfig(); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + nock('https://url').get('/').reply(200, { + name: 'name', + image: 'url', + description: 'description', + }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); await nftController.addNft('0x01', '1234'); @@ -1502,36 +1518,32 @@ describe('NftController', () => { description: 'description', image: 'url', name: 'name', + standard: ERC721, tokenId: '1234', favorite: false, isCurrentlyOwned: true, + tokenURI, }); }); it('should add an nft and nftContract to state when all contract information is falsy and the source is left empty (defaults to "custom")', async () => { + const tokenURI = 'https://url/'; const mockOnNftAdded = jest.fn(); + const mockGetERC721AssetSymbol = jest.fn().mockResolvedValue(''); + const mockGetERC721AssetName = jest.fn().mockResolvedValue(''); + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController } = setupController({ options: { onNftAdded: mockOnNftAdded, + getERC721AssetSymbol: mockGetERC721AssetSymbol, + getERC721AssetName: mockGetERC721AssetName, + getERC721TokenURI: mockGetERC721TokenURI, }, }); - const { selectedAddress, chainId } = nftController.getConfig(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'getNftContractInformation' as any).returns({ - asset_contract_type: null, - created_date: null, - schema_name: null, - symbol: null, - total_supply: null, - description: null, - external_link: null, - collection: { name: null, image_url: null }, - }); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'getNftInformation' as any).returns({ + const { selectedAddress, chainId } = nftController.getConfig(); + nock('https://url').get('/').reply(200, { name: 'name', image: 'url', description: 'description', @@ -1544,6 +1556,7 @@ describe('NftController', () => { [chainId]: [ { address: '0x01234abcdefg', + schemaName: ERC721, }, ], }, @@ -1558,6 +1571,8 @@ describe('NftController', () => { image: 'url', name: 'name', tokenId: '1234', + standard: ERC721, + tokenURI, favorite: false, isCurrentlyOwned: true, }, @@ -1568,41 +1583,32 @@ describe('NftController', () => { expect(mockOnNftAdded).toHaveBeenCalledWith({ address: '0x01234abcdefg', tokenId: '1234', - standard: undefined, + standard: ERC721, symbol: undefined, source: Source.Custom, }); }); it('should add an nft and nftContract to state when all contract information is falsy and the source is "dapp"', async () => { + const tokenURI = 'https://url/'; const mockOnNftAdded = jest.fn(); + const mockGetERC721AssetSymbol = jest.fn().mockResolvedValue(''); + const mockGetERC721AssetName = jest.fn().mockResolvedValue(''); + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, changeNetwork } = setupController({ options: { onNftAdded: mockOnNftAdded, + getERC721AssetSymbol: mockGetERC721AssetSymbol, + getERC721AssetName: mockGetERC721AssetName, + getERC721TokenURI: mockGetERC721TokenURI, }, }); - changeNetwork({ selectedNetworkClientId: InfuraNetworkType.goerli }); - - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'getNftContractInformation' as any).returns({ - asset_contract_type: null, - created_date: null, - schema_name: null, - symbol: null, - total_supply: null, - description: null, - external_link: null, - collection: { name: null, image_url: null }, - }); - - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'getNftInformation' as any).returns({ + nock('https://url').get('/').reply(200, { name: 'name', image: 'url', description: 'description', }); + changeNetwork({ selectedNetworkClientId: InfuraNetworkType.goerli }); await nftController.addNft('0x01234abcdefg', '1234', { userAddress: '0x123', @@ -1614,6 +1620,7 @@ describe('NftController', () => { [GOERLI.chainId]: [ { address: '0x01234abcdefg', + schemaName: ERC721, }, ], }, @@ -1629,7 +1636,9 @@ describe('NftController', () => { name: 'name', tokenId: '1234', favorite: false, + standard: ERC721, isCurrentlyOwned: true, + tokenURI, }, ], }, @@ -1638,7 +1647,7 @@ describe('NftController', () => { expect(mockOnNftAdded).toHaveBeenCalledWith({ address: '0x01234abcdefg', tokenId: '1234', - standard: undefined, + standard: ERC721, symbol: undefined, source: Source.Dapp, }); @@ -1725,7 +1734,7 @@ describe('NftController', () => { description: 'Kudos Description', image: 'Kudos image (from proxy API)', name: 'Kudos Name', - standard: 'ERC721', + standard: ERC721, tokenId: ERC721_KUDOS_TOKEN_ID, favorite: false, isCurrentlyOwned: true, @@ -1746,14 +1755,14 @@ describe('NftController', () => { logo: 'Kudos logo (from proxy API)', name: 'Kudos', totalSupply: '10', - schemaName: 'ERC721', + schemaName: ERC721, }, ]); expect(mockOnNftAdded).toHaveBeenCalledWith({ address: ERC721_KUDOSADDRESS, tokenId: ERC721_KUDOS_TOKEN_ID, - standard: 'ERC721', + standard: ERC721, source: Source.Detected, }); }); @@ -1887,7 +1896,7 @@ describe('NftController', () => { address: ERC721_DEPRESSIONIST_ADDRESS, name: "Maltjik.jpg's Depressionists", symbol: 'DPNS', - schemaName: 'ERC721', + schemaName: ERC721, }); expect( nftController.state.allNfts[selectedAddress][chainId][0], @@ -1897,7 +1906,7 @@ describe('NftController', () => { image: 'image', name: 'name', description: 'description', - standard: 'ERC721', + standard: ERC721, favorite: false, isCurrentlyOwned: true, tokenURI: @@ -2172,21 +2181,24 @@ describe('NftController', () => { describe('addNftVerifyOwnership', () => { it('should verify ownership by selected address and add NFT', async () => { - const { nftController, triggerPreferencesStateChange } = - setupController(); + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + + const { nftController, triggerPreferencesStateChange } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); const firstAddress = '0x123'; const secondAddress = '0x321'; const { chainId } = nftController.getConfig(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(true); - - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(true); + nock('https://url').get('/').reply(200, { + name: 'name', + image: 'url', + description: 'description', + }); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, @@ -2212,6 +2224,8 @@ describe('NftController', () => { image: 'url', name: 'name', tokenId: '1234', + standard: ERC721, + tokenURI, favorite: false, isCurrentlyOwned: true, }); @@ -2220,9 +2234,7 @@ describe('NftController', () => { it('should throw an error if selected address is not owner of input NFT', async () => { const { nftController, triggerPreferencesStateChange } = setupController(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); const firstAddress = '0x123'; triggerPreferencesStateChange({ ...getDefaultPreferencesState(), @@ -2236,21 +2248,27 @@ describe('NftController', () => { }); it('should verify ownership by selected address and add NFT by the correct chainId when passed networkClientId', async () => { - const { nftController, triggerPreferencesStateChange } = - setupController(); + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController, triggerPreferencesStateChange } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); const firstAddress = '0x123'; const secondAddress = '0x321'; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(true); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(true); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + nock('https://url') + .get('/') + .reply(200, { + name: 'name', + image: 'url', + description: 'description', + }) + .persist(); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, @@ -2275,9 +2293,11 @@ describe('NftController', () => { description: 'description', image: 'url', name: 'name', + standard: ERC721, tokenId: '1234', favorite: false, isCurrentlyOwned: true, + tokenURI, }); expect( nftController.state.allNfts[secondAddress][GOERLI.chainId][0], @@ -2286,15 +2306,23 @@ describe('NftController', () => { description: 'description', image: 'url', name: 'name', + standard: ERC721, tokenId: '4321', favorite: false, isCurrentlyOwned: true, + tokenURI, }); }); it('should verify ownership by selected address and add NFT by the correct userAddress when passed userAddress', async () => { + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, changeNetwork, triggerPreferencesStateChange } = - setupController(); + setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); // Ensure that the currently selected address is not the same as either of the userAddresses triggerPreferencesStateChange({ ...getDefaultPreferencesState(), @@ -2305,15 +2333,16 @@ describe('NftController', () => { const firstAddress = '0x123'; const secondAddress = '0x321'; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(true); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(true); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + nock('https://url') + .get('/') + .reply(200, { + name: 'name', + image: 'url', + description: 'description', + }) + .persist(); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); await nftController.addNftVerifyOwnership('0x01', '1234', { userAddress: firstAddress, @@ -2332,7 +2361,9 @@ describe('NftController', () => { name: 'name', tokenId: '1234', favorite: false, + standard: ERC721, isCurrentlyOwned: true, + tokenURI, }); expect( nftController.state.allNfts[secondAddress][GOERLI.chainId][0], @@ -2342,8 +2373,10 @@ describe('NftController', () => { image: 'url', name: 'name', tokenId: '4321', + standard: ERC721, favorite: false, isCurrentlyOwned: true, + tokenURI, }); }); }); @@ -2403,14 +2436,19 @@ describe('NftController', () => { }); it('should remove NFT by selected address', async () => { - const { nftController, triggerPreferencesStateChange } = - setupController(); + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController, triggerPreferencesStateChange } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); const { chainId } = nftController.getConfig(); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + nock('https://url').get('/').reply(200, { + name: 'name', + image: 'url', + description: 'description', + }); const firstAddress = '0x123'; const secondAddress = '0x321'; triggerPreferencesStateChange({ @@ -2444,18 +2482,26 @@ describe('NftController', () => { tokenId: '4321', favorite: false, isCurrentlyOwned: true, + tokenURI, + standard: ERC721, }); }); it('should remove NFT by provider type', async () => { - const { nftController, changeNetwork } = setupController(); + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController, changeNetwork } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); const { selectedAddress } = nftController.getConfig(); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftInformation' as any) - .returns({ name: 'name', image: 'url', description: 'description' }); + nock('https://url').get('/').reply(200, { + name: 'name', + image: 'url', + description: 'description', + }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); await nftController.addNft('0x02', '4321'); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.goerli }); @@ -2477,6 +2523,8 @@ describe('NftController', () => { tokenId: '4321', favorite: false, isCurrentlyOwned: true, + tokenURI, + standard: ERC721, }); }); @@ -2697,8 +2745,12 @@ describe('NftController', () => { }); it('should add NFT with null metadata if the ipfs gateway is disabled and opensea is disabled', async () => { - const { nftController, triggerPreferencesStateChange } = - setupController(); + const { nftController, triggerPreferencesStateChange } = setupController({ + options: { + getERC721TokenURI: jest.fn().mockRejectedValue(''), + getERC1155TokenURI: jest.fn().mockResolvedValue('ipfs://*'), + }, + }); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), @@ -2706,12 +2758,6 @@ describe('NftController', () => { openSeaEnabled: false, }); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'getNftURIAndStandard' as any) - .returns(['ipfs://*', ERC1155]); - const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft(ERC1155_NFT_ADDRESS, ERC1155_NFT_ID); @@ -2859,7 +2905,7 @@ describe('NftController', () => { image: 'new_image', name: 'new_name', description: 'new_description', - standard: 'ERC721', + standard: ERC721, }, }, ); @@ -2910,7 +2956,7 @@ describe('NftController', () => { image: 'new_image', name: 'new_name', description: 'new_description', - standard: 'ERC721', + standard: ERC721, }, }, ); @@ -2998,9 +3044,7 @@ describe('NftController', () => { describe('checkAndUpdateAllNftsOwnershipStatus', () => { it('should check whether NFTs for the current selectedAddress/chainId combination are still owned by the selectedAddress and update the isCurrentlyOwned value to false when NFT is not still owned', async () => { const { nftController } = setupController(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x02', '1', { @@ -3027,9 +3071,7 @@ describe('NftController', () => { it('should check whether NFTs for the current selectedAddress/chainId combination are still owned by the selectedAddress and leave/set the isCurrentlyOwned value to true when NFT is still owned', async () => { const { nftController } = setupController(); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(true); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(true); const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x02', '1', { @@ -3056,11 +3098,9 @@ describe('NftController', () => { it('should check whether NFTs for the current selectedAddress/chainId combination are still owned by the selectedAddress and leave the isCurrentlyOwned value as is when NFT ownership check fails', async () => { const { nftController } = setupController(); - sinon - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .stub(nftController, 'isNftOwner' as any) - .throws(new Error('Unable to verify ownership')); + jest + .spyOn(nftController, 'isNftOwner') + .mockRejectedValue('Unable to verify ownership'); const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x02', '1', { @@ -3112,9 +3152,7 @@ describe('NftController', () => { .isCurrentlyOwned, ).toBe(true); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), @@ -3158,9 +3196,7 @@ describe('NftController', () => { .isCurrentlyOwned, ).toBe(true); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); await nftController.checkAndUpdateSingleNftOwnershipStatus(nft, false); @@ -3192,9 +3228,7 @@ describe('NftController', () => { .isCurrentlyOwned, ).toBe(true); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); const updatedNft = await nftController.checkAndUpdateSingleNftOwnershipStatus(nft, true); @@ -3238,9 +3272,7 @@ describe('NftController', () => { .isCurrentlyOwned, ).toBe(true); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), @@ -3291,9 +3323,7 @@ describe('NftController', () => { .isCurrentlyOwned, ).toBe(true); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sinon.stub(nftController, 'isNftOwner' as any).returns(false); + jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), @@ -3499,7 +3529,13 @@ describe('NftController', () => { describe('updateNftMetadata', () => { it('should update Nft metadata successfully', async () => { - const { nftController } = setupController(); + const tokenURI = 'https://api.pudgypenguins.io/lil/4'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); const { selectedAddress } = nftController.getConfig(); const spy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; @@ -3508,13 +3544,11 @@ describe('NftController', () => { networkClientId: testNetworkClientId, }); - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .returns({ - name: 'name pudgy', - image: 'url pudgy', - description: 'description pudgy', - }); + nock('https://api.pudgypenguins.io').get('/lil/4').reply(200, { + name: 'name pudgy', + image: 'url pudgy', + description: 'description pudgy', + }); const testInputNfts: Nft[] = [ { address: '0xtest', @@ -3523,9 +3557,9 @@ describe('NftController', () => { image: null, isCurrentlyOwned: true, name: null, - standard: 'ERC721', + standard: ERC721, tokenId: '3', - tokenURI: 'https://api.pudgypenguins.io/lil/4', + tokenURI, }, ]; @@ -3543,7 +3577,7 @@ describe('NftController', () => { image: 'url pudgy', name: 'name pudgy', tokenId: '3', - standard: 'ERC721', + standard: ERC721, favorite: false, isCurrentlyOwned: true, tokenURI: 'https://api.pudgypenguins.io/lil/4', @@ -3551,27 +3585,35 @@ describe('NftController', () => { }); it('should not update metadata when state nft and fetched nft are the same', async () => { - const { nftController } = setupController(); + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); const { selectedAddress } = nftController.getConfig(); - const spy = jest.spyOn(nftController, 'updateNft'); + const updateNftSpy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; await nftController.addNft('0xtest', '3', { nftMetadata: { name: 'toto', description: 'description', image: 'image.png', - standard: 'ERC721', + standard: ERC721, + tokenURI, }, networkClientId: testNetworkClientId, }); - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .returns({ + nock('https://url') + .get('/') + .reply(200, { name: 'toto', image: 'image.png', description: 'description', - }); + }) + .persist(); const testInputNfts: Nft[] = [ { address: '0xtest', @@ -3580,7 +3622,7 @@ describe('NftController', () => { image: 'image.png', isCurrentlyOwned: true, name: 'toto', - standard: 'ERC721', + standard: ERC721, tokenId: '3', }, ]; @@ -3590,7 +3632,7 @@ describe('NftController', () => { networkClientId: testNetworkClientId, }); - expect(spy).toHaveBeenCalledTimes(0); + expect(updateNftSpy).toHaveBeenCalledTimes(0); expect( nftController.state.allNfts[selectedAddress][SEPOLIA.chainId][0], ).toStrictEqual({ @@ -3600,13 +3642,20 @@ describe('NftController', () => { image: 'image.png', isCurrentlyOwned: true, name: 'toto', - standard: 'ERC721', + standard: ERC721, tokenId: '3', + tokenURI, }); }); it('should trigger update metadata when state nft and fetched nft are not the same', async () => { - const { nftController } = setupController(); + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); + const { nftController } = setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); const { selectedAddress } = nftController.getConfig(); const spy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; @@ -3615,18 +3664,16 @@ describe('NftController', () => { name: 'toto', description: 'description', image: 'image.png', - standard: 'ERC721', + standard: ERC721, }, networkClientId: testNetworkClientId, }); - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .returns({ - name: 'toto', - image: 'image-updated.png', - description: 'description', - }); + nock('https://url').get('/').reply(200, { + name: 'toto', + image: 'image-updated.png', + description: 'description', + }); const testInputNfts: Nft[] = [ { address: '0xtest', @@ -3635,7 +3682,7 @@ describe('NftController', () => { image: 'image.png', isCurrentlyOwned: true, name: 'toto', - standard: 'ERC721', + standard: ERC721, tokenId: '3', }, ]; @@ -3655,190 +3702,12 @@ describe('NftController', () => { 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.getConfig(); - const spy = jest.spyOn(nftController, 'updateNft'); - const testNetworkClientId = 'sepolia'; - await nftController.addNft('0xtest', '3', { - nftMetadata: { - name: '', - description: '', - image: '', - standard: 'ERC721', - }, - networkClientId: testNetworkClientId, - }); - - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .rejects(new Error('Error')); - const testInputNfts: Nft[] = [ - { - address: '0xtest', - description: null, - favorite: false, - image: null, - isCurrentlyOwned: true, - name: null, - 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: '', - favorite: false, - image: '', - isCurrentlyOwned: true, - name: '', - standard: 'ERC721', + standard: ERC721, tokenId: '3', + tokenURI, }); }); - it('should update metadata when some calls to fetch metadata succeed', async () => { - const { nftController } = setupController(); - const { selectedAddress } = nftController.getConfig(); - const spy = jest.spyOn(nftController, 'updateNft'); - const testNetworkClientId = 'sepolia'; - // Add nfts - await nftController.addNft('0xtest1', '1', { - nftMetadata: { - name: '', - description: '', - image: '', - standard: 'ERC721', - }, - networkClientId: testNetworkClientId, - }); - - await nftController.addNft('0xtest2', '2', { - nftMetadata: { - name: '', - description: '', - image: '', - standard: 'ERC721', - }, - networkClientId: testNetworkClientId, - }); - - await nftController.addNft('0xtest3', '3', { - nftMetadata: { - name: '', - description: '', - image: '', - standard: 'ERC721', - }, - networkClientId: testNetworkClientId, - }); - - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .onFirstCall() - .returns({ - name: 'name pudgy 1', - image: 'url pudgy 1', - description: 'description pudgy 2', - }) - .onSecondCall() - .returns({ - name: 'name pudgy 2', - image: 'url pudgy 2', - description: 'description pudgy 2', - }) - .onThirdCall() - .rejects(new Error('Error')); - - const testInputNfts: Nft[] = [ - { - address: '0xtest1', - description: null, - favorite: false, - image: null, - isCurrentlyOwned: true, - name: null, - standard: 'ERC721', - tokenId: '1', - }, - { - address: '0xtest2', - description: null, - favorite: false, - image: null, - isCurrentlyOwned: true, - name: null, - standard: 'ERC721', - tokenId: '2', - }, - { - address: '0xtest3', - description: null, - favorite: false, - image: null, - isCurrentlyOwned: true, - name: null, - standard: 'ERC721', - tokenId: '3', - }, - ]; - - await nftController.updateNftMetadata({ - nfts: testInputNfts, - networkClientId: testNetworkClientId, - }); - - expect(spy).toHaveBeenCalledTimes(2); - expect( - nftController.state.allNfts[selectedAddress][SEPOLIA.chainId], - ).toStrictEqual([ - { - address: '0xtest1', - description: 'description pudgy 2', - favorite: false, - image: 'url pudgy 1', - isCurrentlyOwned: true, - name: 'name pudgy 1', - standard: 'ERC721', - tokenId: '1', - }, - { - address: '0xtest2', - description: 'description pudgy 2', - favorite: false, - image: 'url pudgy 2', - isCurrentlyOwned: true, - name: 'name pudgy 2', - standard: 'ERC721', - tokenId: '2', - }, - { - address: '0xtest3', - tokenId: '3', - favorite: false, - isCurrentlyOwned: true, - name: '', - description: '', - image: '', - standard: 'ERC721', - }, - ]); - }); - it('should not update metadata when nfts has image/name/description already', async () => { const { nftController, triggerPreferencesStateChange } = setupController(); @@ -3851,7 +3720,7 @@ describe('NftController', () => { name: 'test name', description: 'test description', image: 'test image', - standard: 'ERC721', + standard: ERC721, }, userAddress: OWNER_ADDRESS, networkClientId: testNetworkClientId, @@ -3869,8 +3738,14 @@ describe('NftController', () => { }); it('should trigger calling updateNftMetadata when preferences change - openseaEnabled', async () => { + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, triggerPreferencesStateChange, changeNetwork } = - setupController(); + setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); const spy = jest.spyOn(nftController, 'updateNftMetadata'); @@ -3881,7 +3756,7 @@ describe('NftController', () => { name: '', description: '', image: '', - standard: 'ERC721', + standard: ERC721, }, userAddress: OWNER_ADDRESS, networkClientId: testNetworkClientId, @@ -3892,13 +3767,11 @@ describe('NftController', () => { .isCurrentlyOwned, ).toBe(true); - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .returns({ - name: 'name pudgy', - image: 'url pudgy', - description: 'description pudgy', - }); + nock('https://url').get('/').reply(200, { + name: 'name pudgy', + image: 'url pudgy', + description: 'description pudgy', + }); // trigger preference change triggerPreferencesStateChange({ @@ -3912,8 +3785,14 @@ describe('NftController', () => { }); it('should trigger calling updateNftMetadata when preferences change - ipfs enabled', async () => { + const tokenURI = 'https://url/'; + const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, triggerPreferencesStateChange, changeNetwork } = - setupController(); + setupController({ + options: { + getERC721TokenURI: mockGetERC721TokenURI, + }, + }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); const spy = jest.spyOn(nftController, 'updateNftMetadata'); @@ -3924,7 +3803,7 @@ describe('NftController', () => { name: '', description: '', image: '', - standard: 'ERC721', + standard: ERC721, }, userAddress: OWNER_ADDRESS, networkClientId: testNetworkClientId, @@ -3935,13 +3814,11 @@ describe('NftController', () => { .isCurrentlyOwned, ).toBe(true); - sinon - .stub(nftController, 'getNftInformation' as keyof typeof nftController) - .returns({ - name: 'name pudgy', - image: 'url pudgy', - description: 'description pudgy', - }); + nock('https://url').get('/').reply(200, { + name: 'name pudgy', + image: 'url pudgy', + description: 'description pudgy', + }); // trigger preference change triggerPreferencesStateChange({ diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index 06b6cbe2a52..fb927535398 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -485,7 +485,7 @@ export class NftController extends BaseController< * @param tokenId - The NFT identifier. * @returns Promise resolving to the current NFT name and image. */ - private async getNftInformationFromApi( + async #getNftInformationFromApi( contractAddress: string, tokenId: string, ): Promise { @@ -572,7 +572,7 @@ export class NftController extends BaseController< ): Promise { const { ipfsGateway, useIpfsSubdomains, isIpfsGatewayEnabled } = this.#config; - const result = await this.getNftURIAndStandard( + const result = await this.#getNftURIAndStandard( contractAddress, tokenId, networkClientId, @@ -644,7 +644,7 @@ export class NftController extends BaseController< * @param networkClientId - The networkClientId that can be used to identify the network client to use for this request. * @returns Promise resolving NFT uri and token standard. */ - async getNftURIAndStandard( + async #getNftURIAndStandard( contractAddress: string, tokenId: string, networkClientId?: NetworkClientId, @@ -698,7 +698,7 @@ export class NftController extends BaseController< * @param networkClientId - The networkClientId that can be used to identify the network client to use for this request. * @returns Promise resolving to the current NFT name and image. */ - private async getNftInformation( + async #getNftInformation( contractAddress: string, tokenId: string, networkClientId?: NetworkClientId, @@ -716,7 +716,7 @@ export class NftController extends BaseController< ), this.#config.openSeaEnabled && chainId === '0x1' ? safelyExecute(() => - this.getNftInformationFromApi(contractAddress, tokenId), + this.#getNftInformationFromApi(contractAddress, tokenId), ) : undefined, ]); @@ -767,7 +767,7 @@ export class NftController extends BaseController< * @param networkClientId - The networkClientId that can be used to identify the network client to use for this request. * @returns Promise resolving to the NFT contract name, image and description. */ - private async getNftContractInformation( + async #getNftContractInformation( contractAddress: string, nftMetadataFromApi: NftMetadata, networkClientId?: NetworkClientId, @@ -951,7 +951,7 @@ export class NftController extends BaseController< // this doesn't work currently for detection if the user switches networks while the detection is processing // will be fixed once detection uses networkClientIds // get name and symbol if ERC721 then put together the metadata - const contractInformation = await this.getNftContractInformation( + const contractInformation = await this.#getNftContractInformation( checksumHexAddress, nftMetadata, networkClientId, @@ -1224,7 +1224,7 @@ export class NftController extends BaseController< ) { await this.#validateWatchNft(asset, type, userAddress); - const nftMetadata = await this.getNftInformation( + const nftMetadata = await this.#getNftInformation( asset.address, asset.tokenId, networkClientId, @@ -1395,7 +1395,7 @@ export class NftController extends BaseController< nftMetadata = nftMetadata || - (await this.getNftInformation( + (await this.#getNftInformation( checksumHexAddress, tokenId, networkClientId, @@ -1454,9 +1454,9 @@ export class NftController extends BaseController< address: toChecksumHexAddress(nft.address), }; }); - const nftMetadataResults = await Promise.allSettled( + const nftMetadataResults = await Promise.all( nftsWithChecksumAdr.map(async (nft) => { - const resMetadata = await this.getNftInformation( + const resMetadata = await this.#getNftInformation( nft.address, nft.tokenId, networkClientId, @@ -1467,26 +1467,22 @@ export class NftController extends BaseController< }; }), ); - const successfulNewFetchedNfts = nftMetadataResults.filter( - (result): result is PromiseFulfilledResult => - result.status === 'fulfilled', - ); + // We want to avoid updating the state if the state and fetched nft info are the same - const nftsWithDifferentMetadata: PromiseFulfilledResult[] = []; + const nftsWithDifferentMetadata: NftUpdate[] = []; const { allNfts } = this.state; const stateNfts = allNfts[userAddress]?.[chainId] || []; - successfulNewFetchedNfts.forEach((singleNft) => { + nftMetadataResults.forEach((singleNft) => { const existingEntry: Nft | undefined = stateNfts.find( (nft) => - nft.address.toLowerCase() === - singleNft.value.nft.address.toLowerCase() && - nft.tokenId === singleNft.value.nft.tokenId, + nft.address.toLowerCase() === singleNft.nft.address.toLowerCase() && + nft.tokenId === singleNft.nft.tokenId, ); if (existingEntry) { const differentMetadata = compareNftMetadata( - singleNft.value.newMetadata, + singleNft.newMetadata, existingEntry, ); @@ -1498,12 +1494,7 @@ export class NftController extends BaseController< if (nftsWithDifferentMetadata.length !== 0) { nftsWithDifferentMetadata.forEach((elm) => - this.updateNft( - elm.value.nft, - elm.value.newMetadata, - userAddress, - chainId, - ), + this.updateNft(elm.nft, elm.newMetadata, userAddress, chainId), ); } } From 0f3e811c67f1a4eb48a62ad9d89808949cdedc7b Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 29 May 2024 22:19:32 +0200 Subject: [PATCH 09/17] fix: remove AllowedActions & AllowedEvents export --- packages/assets-controllers/src/NftController.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index fb927535398..b8fea999ffd 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -218,11 +218,11 @@ const controllerName = 'NftController'; /** * The external actions available to the {@link NftController}. */ -export type AllowedActions = +type AllowedActions = | AddApprovalRequest | NetworkControllerGetNetworkClientByIdAction; -export type AllowedEvents = +type AllowedEvents = | PreferencesControllerStateChangeEvent | NetworkControllerNetworkDidChangeEvent; From 3f5bdc6dce9b282bff8d9876efc95eaf70368941 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 29 May 2024 23:01:13 +0200 Subject: [PATCH 10/17] fix: minor controller updates --- .../assets-controllers/src/NftController.ts | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index b8fea999ffd..ac11d918206 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -4,7 +4,10 @@ import type { RestrictedControllerMessenger, ControllerStateChangeEvent, } from '@metamask/base-controller'; -import { BaseController } from '@metamask/base-controller'; +import { + BaseController, + type ControllerGetStateAction, +} from '@metamask/base-controller'; import { safelyExecute, handleFetch, @@ -196,7 +199,7 @@ export type NftControllerState = { ignoredNfts: Nft[]; }; -const metadata = { +const nftControllerMetadata = { allNftContracts: { persist: true, anonymous: false }, allNfts: { persist: true, anonymous: false }, ignoredNfts: { persist: true, anonymous: false }, @@ -215,6 +218,12 @@ type NftAsset = { */ const controllerName = 'NftController'; +export type NftControllerGetStateAction = ControllerGetStateAction< + typeof controllerName, + NftControllerState +>; +export type NftControllerActions = NftControllerGetStateAction; + /** * The external actions available to the {@link NftController}. */ @@ -238,7 +247,7 @@ export type NftControllerEvents = NftControllerStateChangeEvent; */ export type NftControllerMessenger = RestrictedControllerMessenger< typeof controllerName, - AllowedActions, + NftControllerActions | AllowedActions, NftControllerEvents | AllowedEvents, AllowedActions['type'], AllowedEvents['type'] @@ -337,7 +346,7 @@ export class NftController extends BaseController< }) { super({ name: controllerName, - metadata, + metadata: nftControllerMetadata, messenger, state: { ...defaultNftControllerState, @@ -404,13 +413,12 @@ export class NftController extends BaseController< openSeaEnabled, isIpfsGatewayEnabled, }: PreferencesState) { - this.#config = { - ...this.#config, + this.setConfig({ selectedAddress, ipfsGateway, openSeaEnabled, isIpfsGatewayEnabled, - }; + }); const needsUpdateNftMetadata = (isIpfsGatewayEnabled && ipfsGateway !== '') || openSeaEnabled; @@ -828,7 +836,7 @@ export class NftController extends BaseController< * @param chainId - The chainId of the network where the NFT is being added. * @param userAddress - The address of the account where the NFT is being added. * @param source - Whether the NFT was detected, added manually or suggested by a dapp. - * @returns Promise resolving to the current NFT list. + * @returns A promise resolving to `undefined`. */ async #addIndividualNft( tokenAddress: string, @@ -886,7 +894,7 @@ export class NftController extends BaseController< nfts.push(newEntry); } - this.#updateNestedNftState(nfts, 'allNfts', { + this.#updateNestedNftState(nfts, ALL_NFTS_STATE_KEY, { chainId, userAddress, }); From 3a9ca6db4138bad57fc5a264be90b335882f1daa Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 30 May 2024 13:11:52 +0200 Subject: [PATCH 11/17] fix: restore default state as a function --- packages/assets-controllers/src/NftController.ts | 6 +++--- .../assets-controllers/src/NftDetectionController.test.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index ac11d918206..4ec68def803 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -253,11 +253,11 @@ export type NftControllerMessenger = RestrictedControllerMessenger< AllowedEvents['type'] >; -export const defaultNftControllerState: NftControllerState = { +export const getDefaultNftControllerState = (): NftControllerState => ({ allNftContracts: {}, allNfts: {}, ignoredNfts: [], -}; +}); /** * Controller that stores assets and exposes convenience methods @@ -349,7 +349,7 @@ export class NftController extends BaseController< metadata: nftControllerMetadata, messenger, state: { - ...defaultNftControllerState, + ...getDefaultNftControllerState(), ...state, }, }); diff --git a/packages/assets-controllers/src/NftDetectionController.test.ts b/packages/assets-controllers/src/NftDetectionController.test.ts index d997d06d578..46337556782 100644 --- a/packages/assets-controllers/src/NftDetectionController.test.ts +++ b/packages/assets-controllers/src/NftDetectionController.test.ts @@ -25,7 +25,7 @@ import { } from '../../network-controller/tests/helpers'; import { Source } from './constants'; import { - defaultNftControllerState, + getDefaultNftControllerState, type NftControllerState, } from './NftController'; import { @@ -774,7 +774,7 @@ describe('NftDetectionController', () => { const mockAddNft = jest.fn(); const mockGetNftState = jest.fn().mockImplementation(() => { return { - ...defaultNftControllerState, + ...getDefaultNftControllerState(), ignoredNfts: [ // This address and token ID are always detected, as determined by // the nock mocks setup in `beforeEach` @@ -1110,7 +1110,7 @@ async function withController( addNft: jest.fn(), getNftApi: jest.fn(), getNetworkClientById, - getNftState: () => defaultNftControllerState, + getNftState: getDefaultNftControllerState, disabled: true, selectedAddress: '', ...options, From 2a5df85482e6d68d05d0b4e9907d9e0235615e54 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 30 May 2024 13:16:01 +0200 Subject: [PATCH 12/17] fix: chainId type --- 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 4ec68def803..cb5bd2b2f57 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -470,7 +470,7 @@ export class NftController extends BaseController< >( newCollection: U, baseStateKey: T, - { userAddress, chainId }: { userAddress: string; chainId: string }, + { userAddress, chainId }: { userAddress: string; chainId: Hex }, ) { this.update((state) => { const oldState = state[baseStateKey]; From ce3390e71f82d8ca9184eecb18125262b65e2fc0 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 30 May 2024 13:17:35 +0200 Subject: [PATCH 13/17] fix: use descriptive generic params --- packages/assets-controllers/src/NftController.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index cb5bd2b2f57..c57cdbd7e34 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -465,11 +465,11 @@ export class NftController extends BaseController< * @param passedConfig.chainId - the chainId passed through the NFT detection flow to ensure assets are stored to the correct account */ #updateNestedNftState< - T extends 'allNfts' | 'allNftContracts', - U extends T extends 'allNfts' ? Nft[] : NftContract[], + Key extends 'allNfts' | 'allNftContracts', + NftCollection extends Key extends 'allNfts' ? Nft[] : NftContract[], >( - newCollection: U, - baseStateKey: T, + newCollection: NftCollection, + baseStateKey: Key, { userAddress, chainId }: { userAddress: string; chainId: Hex }, ) { this.update((state) => { From c4e5db7cc95349c2c182d5d6651a15e5f42ed0cd Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 30 May 2024 13:58:32 +0200 Subject: [PATCH 14/17] fix: merge missing update --- packages/assets-controllers/src/NftDetectionController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/NftDetectionController.ts b/packages/assets-controllers/src/NftDetectionController.ts index 2a7a0574e46..25f7fd6ef4d 100644 --- a/packages/assets-controllers/src/NftDetectionController.ts +++ b/packages/assets-controllers/src/NftDetectionController.ts @@ -361,7 +361,7 @@ export class NftDetectionController extends StaticIntervalPollingController< readonly #addNft: NftController['addNft']; - readonly #getNftState: () => NftState; + readonly #getNftState: () => NftControllerState; /** * The controller options From e14824085025ebd353ef5e1a2c5be9bce9b58232 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 30 May 2024 17:56:56 +0200 Subject: [PATCH 15/17] fix: remove config object in favor of class variables --- .../src/NftController.test.ts | 411 +++++++++++------- .../assets-controllers/src/NftController.ts | 144 +++--- 2 files changed, 319 insertions(+), 236 deletions(-) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 0ee96888e23..7bd4df5a19e 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -1052,13 +1052,15 @@ describe('NftController', () => { describe('addNft', () => { it('should add NFT and NFT contract', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + chainId: ChainId.mainnet, + selectedAddress, getERC721AssetName: jest.fn().mockResolvedValue('Name'), }, }); - const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x01', '1', { nftMetadata: { name: 'name', @@ -1074,7 +1076,7 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x01', description: 'description', @@ -1091,7 +1093,9 @@ describe('NftController', () => { }); expect( - nftController.state.allNftContracts[selectedAddress][chainId][0], + nftController.state.allNftContracts[selectedAddress][ + ChainId.mainnet + ][0], ).toStrictEqual({ address: '0x01', logo: 'url', @@ -1167,7 +1171,6 @@ describe('NftController', () => { getERC1155TokenURI: mockGetERC1155TokenURI, }, }); - const { chainId } = nftController.getConfig(); const firstAddress = '0x123'; const secondAddress = '0x321'; @@ -1194,7 +1197,7 @@ describe('NftController', () => { selectedAddress: firstAddress, }); expect( - nftController.state.allNfts[firstAddress][chainId][0], + nftController.state.allNfts[firstAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x01', description: 'description', @@ -1209,8 +1212,12 @@ describe('NftController', () => { }); it('should update NFT if image is different', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); await nftController.addNft('0x01', '1', { nftMetadata: { @@ -1223,7 +1230,7 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x01', description: 'description', @@ -1246,7 +1253,7 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x01', description: 'description', @@ -1260,8 +1267,13 @@ describe('NftController', () => { }); it('should not duplicate NFT nor NFT contract if already added', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + await nftController.addNft('0x01', '1', { nftMetadata: { name: 'name', @@ -1283,17 +1295,19 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); expect( - nftController.state.allNftContracts[selectedAddress][chainId], + nftController.state.allNftContracts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); }); it('should add NFT and get information from NFT-API', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + selectedAddress, getERC721TokenURI: jest .fn() .mockRejectedValue(new Error('Not an ERC721 contract')), @@ -1303,10 +1317,9 @@ describe('NftController', () => { }, }); - const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x01', '1'); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x01', description: 'Description', @@ -1323,8 +1336,10 @@ describe('NftController', () => { }); it('should add NFT erc721 and aggregate NFT data from both contract and NFT-API', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + selectedAddress, getERC721AssetName: jest.fn().mockResolvedValue('KudosToken'), getERC721AssetSymbol: jest.fn().mockResolvedValue('KDO'), getERC721TokenURI: jest @@ -1359,12 +1374,10 @@ describe('NftController', () => { description: 'Kudos Description (directly from tokenURI)', }); - const { selectedAddress, chainId } = nftController.getConfig(); - await nftController.addNft(ERC721_KUDOSADDRESS, ERC721_KUDOS_TOKEN_ID); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: ERC721_KUDOSADDRESS, image: 'Kudos Image (directly from tokenURI)', @@ -1379,7 +1392,9 @@ describe('NftController', () => { }); expect( - nftController.state.allNftContracts[selectedAddress][chainId][0], + nftController.state.allNftContracts[selectedAddress][ + ChainId.mainnet + ][0], ).toStrictEqual({ address: ERC721_KUDOSADDRESS, name: 'KudosToken', @@ -1389,8 +1404,10 @@ describe('NftController', () => { }); it('should add NFT erc1155 and get NFT information from contract when NFT API call fail', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + selectedAddress, getERC721TokenURI: jest .fn() .mockRejectedValue(new Error('Not a 721 contract')), @@ -1412,11 +1429,11 @@ describe('NftController', () => { image: 'image (directly from tokenURI)', animation_url: null, }); - const { selectedAddress, chainId } = nftController.getConfig(); + await nftController.addNft(ERC1155_NFT_ADDRESS, ERC1155_NFT_ID); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: ERC1155_NFT_ADDRESS, image: 'image (directly from tokenURI)', @@ -1432,8 +1449,10 @@ describe('NftController', () => { }); it('should add NFT erc721 and get NFT information only from contract', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + selectedAddress, getERC721AssetName: jest.fn().mockResolvedValue('KudosToken'), getERC721AssetSymbol: jest.fn().mockResolvedValue('KDO'), getERC721TokenURI: jest.fn().mockImplementation((tokenAddress) => { @@ -1453,7 +1472,7 @@ describe('NftController', () => { name: 'Kudos Name (directly from tokenURI)', description: 'Kudos Description (directly from tokenURI)', }); - const { selectedAddress, chainId } = nftController.getConfig(); + nock('https://nft.api.cx.metamask.io') .get( '/tokens?chainIds=1&tokens=0x2aEa4Add166EBf38b63d09a75dE1a7b94Aa24163%3A1203&includeTopBid=true&includeAttributes=true&includeLastSale=true', @@ -1463,7 +1482,7 @@ describe('NftController', () => { await nftController.addNft(ERC721_KUDOSADDRESS, ERC721_KUDOS_TOKEN_ID); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: ERC721_KUDOSADDRESS, image: 'Kudos Image (directly from tokenURI)', @@ -1478,7 +1497,9 @@ describe('NftController', () => { }); expect( - nftController.state.allNftContracts[selectedAddress][chainId][0], + nftController.state.allNftContracts[selectedAddress][ + ChainId.mainnet + ][0], ).toStrictEqual({ address: ERC721_KUDOSADDRESS, name: 'KudosToken', @@ -1488,14 +1509,15 @@ describe('NftController', () => { }); it('should add NFT by provider type', async () => { + const selectedAddress = OWNER_ADDRESS; const tokenURI = 'https://url/'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, changeNetwork } = setupController({ options: { + selectedAddress, getERC721TokenURI: mockGetERC721TokenURI, }, }); - const { selectedAddress } = nftController.getConfig(); nock('https://url').get('/').reply(200, { name: 'name', image: 'url', @@ -1532,9 +1554,10 @@ describe('NftController', () => { const mockGetERC721AssetSymbol = jest.fn().mockResolvedValue(''); const mockGetERC721AssetName = jest.fn().mockResolvedValue(''); const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); - + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + selectedAddress, onNftAdded: mockOnNftAdded, getERC721AssetSymbol: mockGetERC721AssetSymbol, getERC721AssetName: mockGetERC721AssetName, @@ -1542,7 +1565,6 @@ describe('NftController', () => { }, }); - const { selectedAddress, chainId } = nftController.getConfig(); nock('https://url').get('/').reply(200, { name: 'name', image: 'url', @@ -1553,7 +1575,7 @@ describe('NftController', () => { expect(nftController.state.allNftContracts).toStrictEqual({ [selectedAddress]: { - [chainId]: [ + [ChainId.mainnet]: [ { address: '0x01234abcdefg', schemaName: ERC721, @@ -1564,7 +1586,7 @@ describe('NftController', () => { expect(nftController.state.allNfts).toStrictEqual({ [selectedAddress]: { - [chainId]: [ + [ChainId.mainnet]: [ { address: '0x01234abcdefg', description: 'description', @@ -1654,9 +1676,11 @@ describe('NftController', () => { }); it('should add an nft and nftContract when there is valid contract information and source is "detected"', async () => { + const selectedAddress = OWNER_ADDRESS; const mockOnNftAdded = jest.fn(); const { nftController } = setupController({ options: { + selectedAddress, onNftAdded: mockOnNftAdded, getERC721AssetName: jest .fn() @@ -1687,23 +1711,7 @@ describe('NftController', () => { }, ], }); - /* nock(OPENSEA_PROXY_URL) - .get(`/chain/ethereum/contract/${ERC721_KUDOSADDRESS}`) - .reply(200, { - address: ERC721_KUDOSADDRESS, - chain: 'ethereum', - collection: 'KDO', - contract_standard: 'erc721', - name: 'Kudos', - total_supply: 10, - }) - .get(`/collections/KDO`) - .reply(200, { - description: 'Kudos Description', - image_url: 'Kudos logo (from proxy API)', - }); */ - const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft( '0x6EbeAf8e8E946F0716E6533A6f2cefc83f60e8Ab', '123', @@ -1714,11 +1722,11 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress]?.[chainId], + nftController.state.allNfts[selectedAddress]?.[ChainId.mainnet], ).toBeUndefined(); expect( - nftController.state.allNftContracts[selectedAddress]?.[chainId], + nftController.state.allNftContracts[selectedAddress]?.[ChainId.mainnet], ).toBeUndefined(); await nftController.addNft(ERC721_KUDOSADDRESS, ERC721_KUDOS_TOKEN_ID, { @@ -1727,7 +1735,7 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toStrictEqual([ { address: ERC721_KUDOSADDRESS, @@ -1748,7 +1756,7 @@ describe('NftController', () => { ]); expect( - nftController.state.allNftContracts[selectedAddress][chainId], + nftController.state.allNftContracts[selectedAddress][ChainId.mainnet], ).toStrictEqual([ { address: ERC721_KUDOSADDRESS, @@ -1768,9 +1776,11 @@ describe('NftController', () => { }); it('should not add an nft and nftContract when there is not valid contract information (or an issue fetching it) and source is "detected"', async () => { + const selectedAddress = OWNER_ADDRESS; const mockOnNftAdded = jest.fn(); const { nftController } = setupController({ options: { + selectedAddress, onNftAdded: mockOnNftAdded, getERC721AssetName: jest .fn() @@ -1785,9 +1795,6 @@ describe('NftController', () => { `/tokens?chainIds=1&tokens=${ERC721_KUDOSADDRESS}%3A${ERC721_KUDOS_TOKEN_ID}&includeTopBid=true&includeAttributes=true&includeLastSale=true`, ) .replyWithError(new Error('Failed to fetch')); - - const { selectedAddress } = nftController.getConfig(); - await nftController.addNft( '0x6EbeAf8e8E946F0716E6533A6f2cefc83f60e8Ab', '123', @@ -1807,8 +1814,12 @@ describe('NftController', () => { }); it('should not add duplicate NFTs to the ignoredNfts list', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); await nftController.addNft('0x01', '1', { nftMetadata: { @@ -1829,13 +1840,13 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(2); expect(nftController.state.ignoredNfts).toHaveLength(0); nftController.removeAndIgnoreNft('0x01', '1'); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); expect(nftController.state.ignoredNfts).toHaveLength(1); @@ -1849,19 +1860,20 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(2); expect(nftController.state.ignoredNfts).toHaveLength(1); nftController.removeAndIgnoreNft('0x01', '1'); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); expect(nftController.state.ignoredNfts).toHaveLength(1); }); it('should add NFT with metadata hosted in IPFS', async () => { - const { nftController } = setupController({ + const selectedAddress = OWNER_ADDRESS; + const { nftController, triggerPreferencesStateChange } = setupController({ options: { getERC721AssetName: jest .fn() @@ -1880,10 +1892,11 @@ describe('NftController', () => { .mockRejectedValue(new Error('Not an ERC1155 token')), }, }); - nftController.setConfig({ + triggerPreferencesStateChange({ + ...getDefaultPreferencesState(), + selectedAddress, ipfsGateway: IPFS_DEFAULT_GATEWAY_URL, }); - const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, @@ -1891,7 +1904,9 @@ describe('NftController', () => { ); expect( - nftController.state.allNftContracts[selectedAddress][chainId][0], + nftController.state.allNftContracts[selectedAddress][ + ChainId.mainnet + ][0], ).toStrictEqual({ address: ERC721_DEPRESSIONIST_ADDRESS, name: "Maltjik.jpg's Depressionists", @@ -1899,7 +1914,7 @@ describe('NftController', () => { schemaName: ERC721, }); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: ERC721_DEPRESSIONIST_ADDRESS, tokenId: '36', @@ -1915,6 +1930,7 @@ describe('NftController', () => { }); it('should add NFT erc721 when call to NFT API fail', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController(); nock(NFT_API_BASE_URL) .get( @@ -1922,12 +1938,10 @@ describe('NftController', () => { ) .replyWithError(new Error('Failed to fetch')); - const { selectedAddress, chainId } = nftController.getConfig(); - await nftController.addNft(ERC721_NFT_ADDRESS, ERC721_NFT_ID); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: ERC721_NFT_ADDRESS, image: null, @@ -2191,7 +2205,6 @@ describe('NftController', () => { }); const firstAddress = '0x123'; const secondAddress = '0x321'; - const { chainId } = nftController.getConfig(); jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(true); nock('https://url').get('/').reply(200, { @@ -2217,7 +2230,7 @@ describe('NftController', () => { selectedAddress: firstAddress, }); expect( - nftController.state.allNfts[firstAddress][chainId][0], + nftController.state.allNfts[firstAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x01', description: 'description', @@ -2383,8 +2396,12 @@ describe('NftController', () => { describe('removeNft', () => { it('should remove NFT and NFT contract', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); await nftController.addNft('0x01', '1', { nftMetadata: { @@ -2396,17 +2413,17 @@ describe('NftController', () => { }); nftController.removeNft('0x01', '1'); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(0); expect( - nftController.state.allNftContracts[selectedAddress][chainId], + nftController.state.allNftContracts[selectedAddress][ChainId.mainnet], ).toHaveLength(0); }); it('should not remove NFT contract if NFT still exists', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x01', '1', { nftMetadata: { @@ -2427,11 +2444,11 @@ describe('NftController', () => { }); nftController.removeNft('0x01', '1'); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); expect( - nftController.state.allNftContracts[selectedAddress][chainId], + nftController.state.allNftContracts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); }); @@ -2443,7 +2460,6 @@ describe('NftController', () => { getERC721TokenURI: mockGetERC721TokenURI, }, }); - const { chainId } = nftController.getConfig(); nock('https://url').get('/').reply(200, { name: 'name', image: 'url', @@ -2464,16 +2480,16 @@ describe('NftController', () => { }); await nftController.addNft('0x01', '1234'); nftController.removeNft('0x01', '1234'); - expect(nftController.state.allNfts[secondAddress][chainId]).toHaveLength( - 0, - ); + expect( + nftController.state.allNfts[secondAddress][ChainId.mainnet], + ).toHaveLength(0); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, selectedAddress: firstAddress, }); expect( - nftController.state.allNfts[firstAddress][chainId][0], + nftController.state.allNfts[firstAddress][ChainId.mainnet][0], ).toStrictEqual({ address: '0x02', description: 'description', @@ -2488,14 +2504,15 @@ describe('NftController', () => { }); it('should remove NFT by provider type', async () => { + const selectedAddress = OWNER_ADDRESS; const tokenURI = 'https://url/'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, changeNetwork } = setupController({ options: { + selectedAddress, getERC721TokenURI: mockGetERC721TokenURI, }, }); - const { selectedAddress } = nftController.getConfig(); nock('https://url').get('/').reply(200, { name: 'name', @@ -2588,8 +2605,12 @@ describe('NftController', () => { }); it('should be able to clear the ignoredNfts list', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); await nftController.addNft('0x02', '1', { nftMetadata: { @@ -2601,15 +2622,15 @@ describe('NftController', () => { }, }); - expect(nftController.state.allNfts[selectedAddress][chainId]).toHaveLength( - 1, - ); + expect( + nftController.state.allNfts[selectedAddress][ChainId.mainnet], + ).toHaveLength(1); expect(nftController.state.ignoredNfts).toHaveLength(0); nftController.removeAndIgnoreNft('0x02', '1'); - expect(nftController.state.allNfts[selectedAddress][chainId]).toHaveLength( - 0, - ); + expect( + nftController.state.allNfts[selectedAddress][ChainId.mainnet], + ).toHaveLength(0); expect(nftController.state.ignoredNfts).toHaveLength(1); nftController.clearIgnoredNfts(); @@ -2745,6 +2766,7 @@ describe('NftController', () => { }); it('should add NFT with null metadata if the ipfs gateway is disabled and opensea is disabled', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController, triggerPreferencesStateChange } = setupController({ options: { getERC721TokenURI: jest.fn().mockRejectedValue(''), @@ -2754,16 +2776,15 @@ describe('NftController', () => { triggerPreferencesStateChange({ ...getDefaultPreferencesState(), + selectedAddress, isIpfsGatewayEnabled: false, openSeaEnabled: false, }); - const { selectedAddress, chainId } = nftController.getConfig(); - await nftController.addNft(ERC1155_NFT_ADDRESS, ERC1155_NFT_ID); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual({ address: ERC1155_NFT_ADDRESS, name: null, @@ -2780,8 +2801,13 @@ describe('NftController', () => { describe('updateNftFavoriteStatus', () => { it('should not set NFT as favorite if nft not found', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2795,7 +2821,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ address: ERC721_DEPRESSIONIST_ADDRESS, @@ -2805,8 +2831,13 @@ describe('NftController', () => { ); }); it('should set NFT as favorite', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2820,7 +2851,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ address: ERC721_DEPRESSIONIST_ADDRESS, @@ -2831,8 +2862,13 @@ describe('NftController', () => { }); it('should set NFT as favorite and then unset it', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2846,7 +2882,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ address: ERC721_DEPRESSIONIST_ADDRESS, @@ -2862,7 +2898,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ address: ERC721_DEPRESSIONIST_ADDRESS, @@ -2873,8 +2909,13 @@ describe('NftController', () => { }); it('should keep the favorite status as true after updating metadata', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2888,7 +2929,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ address: ERC721_DEPRESSIONIST_ADDRESS, @@ -2911,7 +2952,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ image: 'new_image', @@ -2925,13 +2966,18 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); }); it('should keep the favorite status as false after updating metadata', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + await nftController.addNft( ERC721_DEPRESSIONIST_ADDRESS, ERC721_DEPRESSIONIST_ID, @@ -2939,7 +2985,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ address: ERC721_DEPRESSIONIST_ADDRESS, @@ -2962,7 +3008,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual( expect.objectContaining({ image: 'new_image', @@ -2976,7 +3022,7 @@ describe('NftController', () => { ); expect( - nftController.state.allNfts[selectedAddress][chainId], + nftController.state.allNfts[selectedAddress][ChainId.mainnet], ).toHaveLength(1); }); @@ -3043,10 +3089,14 @@ describe('NftController', () => { describe('checkAndUpdateNftsOwnershipStatus', () => { describe('checkAndUpdateAllNftsOwnershipStatus', () => { it('should check whether NFTs for the current selectedAddress/chainId combination are still owned by the selectedAddress and update the isCurrentlyOwned value to false when NFT is not still owned', async () => { - const { nftController } = setupController(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); - const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x02', '1', { nftMetadata: { name: 'name', @@ -3056,24 +3106,28 @@ describe('NftController', () => { favorite: false, }, }); - expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); await nftController.checkAndUpdateAllNftsOwnershipStatus(); + expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(false); }); it('should check whether NFTs for the current selectedAddress/chainId combination are still owned by the selectedAddress and leave/set the isCurrentlyOwned value to true when NFT is still owned', async () => { - const { nftController } = setupController(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(true); - const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x02', '1', { nftMetadata: { name: 'name', @@ -3085,24 +3139,28 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); await nftController.checkAndUpdateAllNftsOwnershipStatus(); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); }); it('should check whether NFTs for the current selectedAddress/chainId combination are still owned by the selectedAddress and leave the isCurrentlyOwned value as is when NFT ownership check fails', async () => { - const { nftController } = setupController(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); jest .spyOn(nftController, 'isNftOwner') .mockRejectedValue('Unable to verify ownership'); - const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x02', '1', { nftMetadata: { name: 'name', @@ -3114,29 +3172,29 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); await nftController.checkAndUpdateAllNftsOwnershipStatus(); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); }); it('should check whether NFTs for the current selectedAddress/chainId combination are still owned by the selectedAddress and update the isCurrentlyOwned value to false when NFT is not still owned, when the currently configured selectedAddress/chainId are different from those passed', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController, changeNetwork, triggerPreferencesStateChange } = setupController(); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, - selectedAddress: OWNER_ADDRESS, + selectedAddress, }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); - const { selectedAddress, chainId } = nftController.getConfig(); await nftController.addNft('0x02', '1', { nftMetadata: { name: 'name', @@ -3148,7 +3206,7 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.sepolia][0] .isCurrentlyOwned, ).toBe(true); @@ -3175,8 +3233,13 @@ describe('NftController', () => { describe('checkAndUpdateSingleNftOwnershipStatus', () => { it('should check whether the passed NFT is still owned by the the current selectedAddress/chainId combination and update its isCurrentlyOwned property in state if batch is false and isNftOwner returns false', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + const nft = { address: '0x02', tokenId: '1', @@ -3192,7 +3255,7 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); @@ -3201,14 +3264,19 @@ describe('NftController', () => { await nftController.checkAndUpdateSingleNftOwnershipStatus(nft, false); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(false); }); it('should check whether the passed NFT is still owned by the the current selectedAddress/chainId combination and return the updated NFT object without updating state if batch is true', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); + const nft = { address: '0x02', tokenId: '1', @@ -3224,7 +3292,7 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); @@ -3234,7 +3302,7 @@ describe('NftController', () => { await nftController.checkAndUpdateSingleNftOwnershipStatus(nft, true); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] .isCurrentlyOwned, ).toBe(true); @@ -3242,17 +3310,17 @@ describe('NftController', () => { }); it('should check whether the passed NFT is still owned by the the selectedAddress/chainId combination passed in the accountParams argument and update its isCurrentlyOwned property in state, when the currently configured selectedAddress/chainId are different from those passed', async () => { + const firstSelectedAddress = OWNER_ADDRESS; const { nftController, changeNetwork, triggerPreferencesStateChange } = setupController(); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), openSeaEnabled: true, - selectedAddress: OWNER_ADDRESS, + selectedAddress: firstSelectedAddress, }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); - const { selectedAddress, chainId } = nftController.getConfig(); const nft = { address: '0x02', tokenId: '1', @@ -3268,7 +3336,7 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[firstSelectedAddress][ChainId.sepolia][0] .isCurrentlyOwned, ).toBe(true); @@ -3293,6 +3361,7 @@ describe('NftController', () => { }); it('should check whether the passed NFT is still owned by the the selectedAddress/chainId combination passed in the accountParams argument and return the updated NFT object without updating state, when the currently configured selectedAddress/chainId are different from those passed and batch is true', async () => { + const firstSelectedAddress = OWNER_ADDRESS; const { nftController, changeNetwork, triggerPreferencesStateChange } = setupController(); @@ -3303,7 +3372,6 @@ describe('NftController', () => { }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); - const { selectedAddress, chainId } = nftController.getConfig(); const nft = { address: '0x02', tokenId: '1', @@ -3319,7 +3387,7 @@ describe('NftController', () => { }); expect( - nftController.state.allNfts[selectedAddress][chainId][0] + nftController.state.allNfts[firstSelectedAddress][ChainId.sepolia][0] .isCurrentlyOwned, ).toBe(true); @@ -3367,22 +3435,28 @@ describe('NftController', () => { }; it('should return null if the NFT does not exist in the state', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); expect( nftController.findNftByAddressAndTokenId( mockNft.address, mockNft.tokenId, selectedAddress, - chainId, + ChainId.mainnet, ), ).toBeNull(); }); it('should return the NFT by the address and tokenId', () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + selectedAddress, state: { allNfts: { [OWNER_ADDRESS]: { [ChainId.mainnet]: [mockNft] }, @@ -3390,20 +3464,20 @@ describe('NftController', () => { }, }, }); - const { selectedAddress, chainId } = nftController.getConfig(); expect( nftController.findNftByAddressAndTokenId( mockNft.address, mockNft.tokenId, selectedAddress, - chainId, + ChainId.mainnet, ), ).toStrictEqual({ nft: mockNft, index: 0 }); }); }); describe('updateNftByAddressAndTokenId', () => { + const selectedAddress = OWNER_ADDRESS; const mockTransactionId = '60d36710-b150-11ec-8a49-c377fbd05e27'; const mockNft = { address: '0x02', @@ -3429,6 +3503,7 @@ describe('NftController', () => { it('should update the NFT if the NFT exist', async () => { const { nftController } = setupController({ options: { + selectedAddress, state: { allNfts: { [OWNER_ADDRESS]: { [ChainId.mainnet]: [mockNft] }, @@ -3436,7 +3511,6 @@ describe('NftController', () => { }, }, }); - const { selectedAddress, chainId } = nftController.getConfig(); nftController.updateNft( mockNft, @@ -3444,17 +3518,20 @@ describe('NftController', () => { transactionId: mockTransactionId, }, selectedAddress, - chainId, + ChainId.mainnet, ); expect( - nftController.state.allNfts[selectedAddress][chainId][0], + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0], ).toStrictEqual(expectedMockNft); }); it('should return undefined if the NFT does not exist', () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); expect( nftController.updateNft( @@ -3463,7 +3540,7 @@ describe('NftController', () => { transactionId: mockTransactionId, }, selectedAddress, - chainId, + ChainId.mainnet, ), ).toBeUndefined(); }); @@ -3485,21 +3562,27 @@ describe('NftController', () => { }; it('should not update any NFT state and should return false when passed a transaction id that does not match that of any NFT', async () => { - const { nftController } = setupController(); - const { selectedAddress, chainId } = nftController.getConfig(); + const selectedAddress = OWNER_ADDRESS; + const { nftController } = setupController({ + options: { + selectedAddress, + }, + }); expect( nftController.resetNftTransactionStatusByTransactionId( nonExistTransactionId, selectedAddress, - chainId, + ChainId.mainnet, ), ).toBe(false); }); it('should set the transaction id of an NFT in state to undefined, and return true when it has successfully updated this state', async () => { + const selectedAddress = OWNER_ADDRESS; const { nftController } = setupController({ options: { + selectedAddress, state: { allNfts: { [OWNER_ADDRESS]: { [ChainId.mainnet]: [mockNft] }, @@ -3507,36 +3590,38 @@ describe('NftController', () => { }, }, }); - const { selectedAddress, chainId } = nftController.getConfig(); expect( - nftController.state.allNfts[selectedAddress][chainId][0].transactionId, + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] + .transactionId, ).toBe(mockTransactionId); expect( nftController.resetNftTransactionStatusByTransactionId( mockTransactionId, selectedAddress, - chainId, + ChainId.mainnet, ), ).toBe(true); expect( - nftController.state.allNfts[selectedAddress][chainId][0].transactionId, + nftController.state.allNfts[selectedAddress][ChainId.mainnet][0] + .transactionId, ).toBeUndefined(); }); }); describe('updateNftMetadata', () => { it('should update Nft metadata successfully', async () => { + const selectedAddress = OWNER_ADDRESS; const tokenURI = 'https://api.pudgypenguins.io/lil/4'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController } = setupController({ options: { + selectedAddress, getERC721TokenURI: mockGetERC721TokenURI, }, }); - const { selectedAddress } = nftController.getConfig(); const spy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; await nftController.addNft('0xtest', '3', { @@ -3585,14 +3670,15 @@ describe('NftController', () => { }); it('should not update metadata when state nft and fetched nft are the same', async () => { + const selectedAddress = OWNER_ADDRESS; const tokenURI = 'https://url/'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController } = setupController({ options: { + selectedAddress, getERC721TokenURI: mockGetERC721TokenURI, }, }); - const { selectedAddress } = nftController.getConfig(); const updateNftSpy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; await nftController.addNft('0xtest', '3', { @@ -3649,14 +3735,15 @@ describe('NftController', () => { }); it('should trigger update metadata when state nft and fetched nft are not the same', async () => { + const selectedAddress = OWNER_ADDRESS; const tokenURI = 'https://url/'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController } = setupController({ options: { + selectedAddress, getERC721TokenURI: mockGetERC721TokenURI, }, }); - const { selectedAddress } = nftController.getConfig(); const spy = jest.spyOn(nftController, 'updateNft'); const testNetworkClientId = 'sepolia'; await nftController.addNft('0xtest', '3', { diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index c57cdbd7e34..8d9df32fde4 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -162,21 +162,6 @@ export type NftMetadata = { rarityRank?: string; }; -/** - * @type NftConfig - * - * NFT controller configuration - * @property selectedAddress - Vault selected address - */ -export type NftConfig = { - selectedAddress: string; - chainId: Hex; - ipfsGateway: string; - openSeaEnabled: boolean; - useIpfsSubdomains: boolean; - isIpfsGatewayEnabled: boolean; -}; - /** * @type NftControllerState * @@ -274,7 +259,17 @@ export class NftController extends BaseController< */ openSeaApiKey?: string; - #config: NftConfig; + #selectedAddress: string; + + #chainId: Hex; + + #ipfsGateway: string; + + #openSeaEnabled: boolean; + + #useIpfsSubdomains: boolean; + + #isIpfsGatewayEnabled: boolean; readonly #getERC721AssetName: AssetsContractController['getERC721AssetName']; @@ -301,6 +296,11 @@ export class NftController extends BaseController< * * @param options - The controller options. * @param options.chainId - The chain ID of the current network. + * @param options.selectedAddress - The currently selected address. + * @param options.ipfsGateway - The configured IPFS gateway. + * @param options.openSeaEnabled - Controls whether the OpenSea API is used. + * @param options.useIpfsSubdomains - Controls whether IPFS subdomains are used. + * @param options.isIpfsGatewayEnabled - Controls whether IPFS is enabled or not. * @param options.getERC721AssetName - Gets the name of the asset at the given address. * @param options.getERC721AssetSymbol - Gets the symbol of the asset at the given address. * @param options.getERC721TokenURI - Gets the URI of the ERC721 token at the given address, with the given ID. @@ -310,11 +310,15 @@ export class NftController extends BaseController< * @param options.onNftAdded - Callback that is called when an NFT is added. Currently used pass data * for tracking the NFT added event. * @param options.messenger - The controller messenger. - * @param options.config - Initial options used to configure this controller. * @param options.state - Initial state to set on this controller. */ constructor({ chainId: initialChainId, + selectedAddress = '', + ipfsGateway = IPFS_DEFAULT_GATEWAY_URL, + openSeaEnabled = false, + useIpfsSubdomains = true, + isIpfsGatewayEnabled = true, getERC721AssetName, getERC721AssetSymbol, getERC721TokenURI, @@ -323,10 +327,14 @@ export class NftController extends BaseController< getERC1155TokenURI, onNftAdded, messenger, - config = {}, state = {}, }: { chainId: Hex; + selectedAddress?: string; + ipfsGateway?: string; + openSeaEnabled?: boolean; + useIpfsSubdomains?: boolean; + isIpfsGatewayEnabled?: boolean; getERC721AssetName: AssetsContractController['getERC721AssetName']; getERC721AssetSymbol: AssetsContractController['getERC721AssetSymbol']; getERC721TokenURI: AssetsContractController['getERC721TokenURI']; @@ -341,7 +349,6 @@ export class NftController extends BaseController< source: string; }) => void; messenger: NftControllerMessenger; - config?: Partial; state?: Partial; }) { super({ @@ -353,15 +360,13 @@ export class NftController extends BaseController< ...state, }, }); - this.#config = { - selectedAddress: '', - chainId: initialChainId, - ipfsGateway: IPFS_DEFAULT_GATEWAY_URL, - openSeaEnabled: false, - useIpfsSubdomains: true, - isIpfsGatewayEnabled: true, - ...config, - }; + + this.#selectedAddress = selectedAddress; + this.#chainId = initialChainId; + this.#ipfsGateway = ipfsGateway; + this.#openSeaEnabled = openSeaEnabled; + this.#useIpfsSubdomains = useIpfsSubdomains; + this.#isIpfsGatewayEnabled = isIpfsGatewayEnabled; this.#getERC721AssetName = getERC721AssetName; this.#getERC721AssetSymbol = getERC721AssetSymbol; @@ -396,7 +401,7 @@ export class NftController extends BaseController< 'NetworkController:getNetworkClientById', selectedNetworkClientId, ); - this.#config.chainId = chainId; + this.#chainId = chainId; } /** @@ -413,19 +418,17 @@ export class NftController extends BaseController< openSeaEnabled, isIpfsGatewayEnabled, }: PreferencesState) { - this.setConfig({ - selectedAddress, - ipfsGateway, - openSeaEnabled, - isIpfsGatewayEnabled, - }); + this.#selectedAddress = selectedAddress; + this.#ipfsGateway = ipfsGateway; + this.#openSeaEnabled = openSeaEnabled; + this.#isIpfsGatewayEnabled = isIpfsGatewayEnabled; const needsUpdateNftMetadata = (isIpfsGatewayEnabled && ipfsGateway !== '') || openSeaEnabled; if (needsUpdateNftMetadata) { - const { chainId } = this.#config; - const nfts: Nft[] = this.state.allNfts[selectedAddress]?.[chainId] ?? []; + const nfts: Nft[] = + this.state.allNfts[selectedAddress]?.[this.#chainId] ?? []; // filter only nfts const nftsToUpdate = nfts.filter( (singleNft) => @@ -444,17 +447,6 @@ export class NftController extends BaseController< return `${NFT_API_BASE_URL}/tokens`; } - getConfig(): Readonly { - return this.#config; - } - - setConfig(config: Partial): void { - this.#config = { - ...this.#config, - ...config, - }; - } - /** * Helper method to update nested state for allNfts and allNftContracts. * @@ -465,8 +457,10 @@ export class NftController extends BaseController< * @param passedConfig.chainId - the chainId passed through the NFT detection flow to ensure assets are stored to the correct account */ #updateNestedNftState< - Key extends 'allNfts' | 'allNftContracts', - NftCollection extends Key extends 'allNfts' ? Nft[] : NftContract[], + Key extends typeof ALL_NFTS_STATE_KEY | typeof ALL_NFTS_CONTRACTS_STATE_KEY, + NftCollection extends Key extends typeof ALL_NFTS_STATE_KEY + ? Nft[] + : NftContract[], >( newCollection: NftCollection, baseStateKey: Key, @@ -578,8 +572,6 @@ export class NftController extends BaseController< tokenId: string, networkClientId?: NetworkClientId, ): Promise { - const { ipfsGateway, useIpfsSubdomains, isIpfsGatewayEnabled } = - this.#config; const result = await this.#getNftURIAndStandard( contractAddress, tokenId, @@ -590,7 +582,7 @@ export class NftController extends BaseController< const hasIpfsTokenURI = tokenURI.startsWith('ipfs://'); - if (hasIpfsTokenURI && !isIpfsGatewayEnabled) { + if (hasIpfsTokenURI && !this.#isIpfsGatewayEnabled) { return { image: null, name: null, @@ -601,7 +593,7 @@ export class NftController extends BaseController< }; } - const isDisplayNFTMediaToggleEnabled = this.#config.openSeaEnabled; + const isDisplayNFTMediaToggleEnabled = this.#openSeaEnabled; if (!hasIpfsTokenURI && !isDisplayNFTMediaToggleEnabled) { return { image: null, @@ -614,7 +606,11 @@ export class NftController extends BaseController< } if (hasIpfsTokenURI) { - tokenURI = getFormattedIpfsUrl(ipfsGateway, tokenURI, useIpfsSubdomains); + tokenURI = getFormattedIpfsUrl( + this.#ipfsGateway, + tokenURI, + this.#useIpfsSubdomains, + ); } try { @@ -722,7 +718,7 @@ export class NftController extends BaseController< networkClientId, ), ), - this.#config.openSeaEnabled && chainId === '0x1' + this.#openSeaEnabled && chainId === '0x1' ? safelyExecute(() => this.#getNftInformationFromApi(contractAddress, tokenId), ) @@ -1199,7 +1195,7 @@ export class NftController extends BaseController< ); return chainId; } - return this.#config.chainId; + return this.#chainId; } /** @@ -1222,12 +1218,12 @@ export class NftController extends BaseController< origin: string, { networkClientId, - userAddress = this.#config.selectedAddress, + userAddress = this.#selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string; } = { - userAddress: this.#config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { await this.#validateWatchNft(asset, type, userAddress); @@ -1345,7 +1341,7 @@ export class NftController extends BaseController< address: string, tokenId: string, { - userAddress = this.#config.selectedAddress, + userAddress = this.#selectedAddress, networkClientId, source, }: { @@ -1353,7 +1349,7 @@ export class NftController extends BaseController< networkClientId?: NetworkClientId; source?: Source; } = { - userAddress: this.#config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { if ( @@ -1387,7 +1383,7 @@ export class NftController extends BaseController< tokenId: string, { nftMetadata, - userAddress = this.#config.selectedAddress, + userAddress = this.#selectedAddress, source = Source.Custom, networkClientId, }: { @@ -1395,7 +1391,7 @@ export class NftController extends BaseController< userAddress?: string; source?: Source; networkClientId?: NetworkClientId; - } = { userAddress: this.#config.selectedAddress }, + } = { userAddress: this.#selectedAddress }, ) { const checksumHexAddress = toChecksumHexAddress(tokenAddress); @@ -1447,7 +1443,7 @@ export class NftController extends BaseController< */ async updateNftMetadata({ nfts, - userAddress = this.#config.selectedAddress, + userAddress = this.#selectedAddress, networkClientId, }: { nfts: Nft[]; @@ -1521,9 +1517,9 @@ export class NftController extends BaseController< tokenId: string, { networkClientId, - userAddress = this.#config.selectedAddress, + userAddress = this.#selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string } = { - userAddress: this.#config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { const chainId = this.#getCorrectChainId({ networkClientId }); @@ -1557,9 +1553,9 @@ export class NftController extends BaseController< tokenId: string, { networkClientId, - userAddress = this.#config.selectedAddress, + userAddress = this.#selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string } = { - userAddress: this.#config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { const chainId = this.#getCorrectChainId({ networkClientId }); @@ -1602,10 +1598,10 @@ export class NftController extends BaseController< nft: Nft, batch: boolean, { - userAddress = this.#config.selectedAddress, + userAddress = this.#selectedAddress, networkClientId, }: { networkClientId?: NetworkClientId; userAddress?: string } = { - userAddress: this.#config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { const chainId = this.#getCorrectChainId({ networkClientId }); @@ -1669,9 +1665,9 @@ export class NftController extends BaseController< async checkAndUpdateAllNftsOwnershipStatus( { networkClientId, - userAddress = this.#config.selectedAddress, + userAddress = this.#selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string } = { - userAddress: this.#config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { const chainId = this.#getCorrectChainId({ networkClientId }); @@ -1710,12 +1706,12 @@ export class NftController extends BaseController< favorite: boolean, { networkClientId, - userAddress = this.#config.selectedAddress, + userAddress = this.#selectedAddress, }: { networkClientId?: NetworkClientId; userAddress?: string; } = { - userAddress: this.#config.selectedAddress, + userAddress: this.#selectedAddress, }, ) { const chainId = this.#getCorrectChainId({ networkClientId }); From 918a154dcf22eb8be2c13eeccd95bb7c3afd9ef1 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 30 May 2024 20:17:21 +0200 Subject: [PATCH 16/17] fix: use named exports --- .../src/NftController.test.ts | 22 +++++++++---------- .../assets-controllers/src/NftController.ts | 4 ++-- packages/assets-controllers/src/index.ts | 13 ++++++++++- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 7bd4df5a19e..2c5004e1c3a 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -1,8 +1,5 @@ import type { Network } from '@ethersproject/providers'; -import type { - ApprovalStateChange, - GetApprovalsState, -} from '@metamask/approval-controller'; +import type { ApprovalControllerMessenger } from '@metamask/approval-controller'; import { ApprovalController } from '@metamask/approval-controller'; import { ControllerMessenger } from '@metamask/base-controller'; import { @@ -21,12 +18,10 @@ import { import type { NetworkClientConfiguration, NetworkClientId, - NetworkControllerNetworkDidChangeEvent, } from '@metamask/network-controller'; import { defaultState as defaultNetworkState } from '@metamask/network-controller'; import { getDefaultPreferencesState, - type PreferencesControllerStateChangeEvent, type PreferencesState, } from '@metamask/preferences-controller'; import BN from 'bn.js'; @@ -49,7 +44,11 @@ import type { NftControllerState, NftControllerMessenger, } from './NftController'; -import { NftController } from './NftController'; +import { + NftController, + type AllowedActions, + type AllowedEvents, +} from './NftController'; const CRYPTOPUNK_ADDRESS = '0xb47e3cd837dDF8e4c57F05d70Ab865de6e193BBB'; const ERC721_KUDOSADDRESS = '0x2aEa4Add166EBf38b63d09a75dE1a7b94Aa24163'; @@ -134,11 +133,12 @@ function setupController({ >; } = {}) { const messenger = new ControllerMessenger< - ExtractAvailableAction | GetApprovalsState, + | ExtractAvailableAction + | AllowedActions + | ExtractAvailableAction, | ExtractAvailableEvent - | ApprovalStateChange - | PreferencesControllerStateChangeEvent - | NetworkControllerNetworkDidChangeEvent + | AllowedEvents + | ExtractAvailableEvent >(); const getNetworkClientById = buildMockGetNetworkClientById( diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index 897215b185c..5f805825e80 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -212,11 +212,11 @@ export type NftControllerActions = NftControllerGetStateAction; /** * The external actions available to the {@link NftController}. */ -type AllowedActions = +export type AllowedActions = | AddApprovalRequest | NetworkControllerGetNetworkClientByIdAction; -type AllowedEvents = +export type AllowedEvents = | PreferencesControllerStateChangeEvent | NetworkControllerNetworkDidChangeEvent; diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index 1322c58d392..d7387825f68 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -1,7 +1,18 @@ export * from './AccountTrackerController'; export * from './AssetsContractController'; export * from './CurrencyRateController'; -export * from './NftController'; +export type { + NftControllerState, + NftControllerMessenger, + NftControllerActions, + NftControllerGetStateAction, + NftControllerEvents, + NftControllerStateChangeEvent, + Nft, + NftContract, + NftMetadata, +} from './NftController'; +export { getDefaultNftControllerState, NftController } from './NftController'; export * from './NftDetectionController'; export type { TokenBalancesControllerMessenger, From 286d0fba964c3e4e1fdd9f6aa01716c0f131fc6c Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 30 May 2024 20:20:02 +0200 Subject: [PATCH 17/17] fix: remove useless as const --- 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 5f805825e80..83fdeae592e 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -201,7 +201,7 @@ type NftAsset = { /** * The name of the {@link NftController}. */ -const controllerName = 'NftController' as const; +const controllerName = 'NftController'; export type NftControllerGetStateAction = ControllerGetStateAction< typeof controllerName,