From 528860562691f98afc29169a526acb9d2099f51e Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 11 Jun 2024 11:33:02 -0400 Subject: [PATCH 01/40] Convert `AssetsContractController` to use messenger pattern without inheriting from `BaseControllerV2` --- .../src/AssetsContractController.ts | 171 ++++++++---------- 1 file changed, 77 insertions(+), 94 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 24871c48a6..7830bbe340 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -1,15 +1,14 @@ import { Contract } from '@ethersproject/contracts'; import { Web3Provider } from '@ethersproject/providers'; -import type { BaseConfig, BaseState } from '@metamask/base-controller'; -import { BaseControllerV1 } from '@metamask/base-controller'; +import type { RestrictedControllerMessenger } from '@metamask/base-controller'; import { IPFS_DEFAULT_GATEWAY_URL } from '@metamask/controller-utils'; import type { NetworkClientId, - NetworkState, - NetworkController, + NetworkControllerGetNetworkClientByIdAction, + NetworkControllerNetworkDidChangeEvent, Provider, } from '@metamask/network-controller'; -import type { PreferencesState } from '@metamask/preferences-controller'; +import type { PreferencesControllerStateChangeEvent } from '@metamask/preferences-controller'; import type { Hex } from '@metamask/utils'; import type BN from 'bn.js'; import abiSingleCallBalancesContract from 'single-call-balance-checker-abi'; @@ -67,105 +66,85 @@ export const SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID: Record = { export const MISSING_PROVIDER_ERROR = 'AssetsContractController failed to set the provider correctly. A provider must be set for this method to be available'; -/** - * @type AssetsContractConfig - * - * Assets Contract controller configuration - * @property provider - Provider used to create a new web3 instance - */ -// 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 AssetsContractConfig extends BaseConfig { - provider: Provider | undefined; - ipfsGateway: string; - chainId: Hex; -} - /** * @type BalanceMap * * Key value object containing the balance for each tokenAddress * @property [tokenAddress] - Address of the token */ -// 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 BalanceMap { +export type BalanceMap = { [tokenAddress: string]: BN; -} +}; + +export const name = 'AssetsContractController'; + +export type AllowedActions = NetworkControllerGetNetworkClientByIdAction; + +export type AllowedEvents = + | PreferencesControllerStateChangeEvent + | NetworkControllerNetworkDidChangeEvent; + +export type AssetsContractControllerMessenger = RestrictedControllerMessenger< + typeof name, + AllowedActions, + AllowedEvents, + AllowedActions['type'], + AllowedEvents['type'] +>; /** * Controller that interacts with contracts on mainnet through web3 */ -export class AssetsContractController extends BaseControllerV1< - AssetsContractConfig, - BaseState -> { - private _provider?: Provider; +export class AssetsContractController { + protected messagingSystem: AssetsContractControllerMessenger; - /** - * Name of this controller used during composition - */ - override name = 'AssetsContractController' as const; + #provider: Provider | undefined; - private readonly getNetworkClientById: NetworkController['getNetworkClientById']; + ipfsGateway: string; + + chainId: Hex; /** * Creates a AssetsContractController instance. * * @param options - The controller options. + * @param options.messenger - * @param options.chainId - The chain ID of the current network. - * @param options.onPreferencesStateChange - Allows subscribing to preference controller state changes. - * @param options.onNetworkDidChange - Allows subscribing to network controller networkDidChange events. - * @param options.getNetworkClientById - Gets the network client with the given id from the NetworkController. - * @param config - Initial options used to configure this controller. - * @param state - Initial state to set on this controller. */ - constructor( - { - chainId: initialChainId, - onPreferencesStateChange, - onNetworkDidChange, - getNetworkClientById, - }: { - chainId: Hex; - onPreferencesStateChange: ( - listener: (preferencesState: PreferencesState) => void, - ) => void; - onNetworkDidChange: ( - listener: (networkState: NetworkState) => void, - ) => void; - getNetworkClientById: NetworkController['getNetworkClientById']; - }, - config?: Partial, - state?: Partial, - ) { - super(config, state); - this.defaultConfig = { - provider: undefined, - ipfsGateway: IPFS_DEFAULT_GATEWAY_URL, - chainId: initialChainId, - }; - this.initialize(); - this.getNetworkClientById = getNetworkClientById; - - onPreferencesStateChange(({ ipfsGateway }) => { - this.configure({ ipfsGateway }); - }); - - onNetworkDidChange(({ selectedNetworkClientId }) => { - const selectedNetworkClient = getNetworkClientById( - selectedNetworkClientId, - ); - const { chainId } = selectedNetworkClient.configuration; - - if (this.config.chainId !== chainId) { - this.configure({ - chainId: selectedNetworkClient.configuration.chainId, - }); - } - }); + constructor({ + messenger, + chainId: initialChainId, + }: { + messenger: AssetsContractControllerMessenger; + chainId: Hex; + }) { + this.chainId = initialChainId; + this.#provider = undefined; + this.ipfsGateway = IPFS_DEFAULT_GATEWAY_URL; + this.messagingSystem = messenger; + + this.messagingSystem.subscribe( + `PreferencesController:stateChange`, + ({ ipfsGateway }) => { + this.ipfsGateway = ipfsGateway; + }, + ); + + this.messagingSystem.subscribe( + `NetworkController:networkDidChange`, + ({ selectedNetworkClientId }) => { + const { + configuration: { chainId }, + } = this.messagingSystem.call( + `NetworkController:getNetworkClientById`, + selectedNetworkClientId, + ); + + if (this.chainId !== chainId) { + this.chainId = chainId; + } + }, + ); } /** @@ -175,8 +154,8 @@ export class AssetsContractController extends BaseControllerV1< * * @property provider - Provider used to create a new underlying Web3 instance */ - set provider(provider: Provider) { - this._provider = provider; + set provider(provider: Provider | undefined) { + this.#provider = provider; } get provider() { @@ -191,8 +170,11 @@ export class AssetsContractController extends BaseControllerV1< */ getProvider(networkClientId?: NetworkClientId): Web3Provider { const provider = networkClientId - ? this.getNetworkClientById(networkClientId).provider - : this._provider; + ? this.messagingSystem.call( + `NetworkController:getNetworkClientById`, + networkClientId, + ).provider + : this.#provider; if (provider === undefined) { throw new Error(MISSING_PROVIDER_ERROR); @@ -209,8 +191,11 @@ export class AssetsContractController extends BaseControllerV1< */ getChainId(networkClientId?: NetworkClientId): Hex { return networkClientId - ? this.getNetworkClientById(networkClientId).configuration.chainId - : this.config.chainId; + ? this.messagingSystem.call( + `NetworkController:getNetworkClientById`, + networkClientId, + ).configuration.chainId + : this.chainId; } /** @@ -337,15 +322,13 @@ export class AssetsContractController extends BaseControllerV1< // Asserts provider is available this.getProvider(networkClientId); - const { ipfsGateway } = this.config; - // ERC721 try { const erc721Standard = this.getERC721Standard(networkClientId); return { ...(await erc721Standard.getDetails( tokenAddress, - ipfsGateway, + this.ipfsGateway, tokenId, )), }; @@ -359,7 +342,7 @@ export class AssetsContractController extends BaseControllerV1< return { ...(await erc1155Standard.getDetails( tokenAddress, - ipfsGateway, + this.ipfsGateway, tokenId, )), }; From 788628034ccf414b941db8c47996aff736641150 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 11 Jun 2024 11:33:25 -0400 Subject: [PATCH 02/40] Fix method returning promise into async function --- packages/assets-controllers/src/AssetsContractController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 7830bbe340..fecd96ca2a 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -287,7 +287,7 @@ export class AssetsContractController { * @param networkClientId - Network Client ID to fetch the provider with. * @returns Promise resolving to token identifier for the 'index'th asset assigned to 'selectedAddress'. */ - getERC721NftTokenId( + async getERC721NftTokenId( address: string, selectedAddress: string, index: number, From 7b2ed468d337a53998675d99632ea936aa02a2df Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 11 Jun 2024 11:33:44 -0400 Subject: [PATCH 03/40] Explicitly enumerate package-level exports --- packages/assets-controllers/src/index.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index e5fb477a02..1888365935 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -8,7 +8,14 @@ export type { AccountTrackerControllerEvents, } from './AccountTrackerController'; export { AccountTrackerController } from './AccountTrackerController'; -export * from './AssetsContractController'; +export type { + BalanceMap, + AssetsContractControllerMessenger, +} from './AssetsContractController'; +export { + SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID, + AssetsContractController, +} from './AssetsContractController'; export * from './CurrencyRateController'; export type { NftControllerState, From fa059a91897bc2a2bbb0862c8cee41fda15b2cba Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 11 Jun 2024 11:39:41 -0400 Subject: [PATCH 04/40] Adapt tests to use messenger pattern --- .../src/AssetsContractController.test.ts | 160 ++++++++++-------- 1 file changed, 87 insertions(+), 73 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.test.ts b/packages/assets-controllers/src/AssetsContractController.test.ts index 8600501e0e..f1c2319d5d 100644 --- a/packages/assets-controllers/src/AssetsContractController.test.ts +++ b/packages/assets-controllers/src/AssetsContractController.test.ts @@ -9,22 +9,24 @@ import { } from '@metamask/controller-utils'; import HttpProvider from '@metamask/ethjs-provider-http'; import type { - NetworkClientId, - NetworkControllerMessenger, Provider, + NetworkClientId, + NetworkControllerEvents, + NetworkControllerActions, } from '@metamask/network-controller'; import { NetworkController, NetworkClientType, } from '@metamask/network-controller'; -import { - getDefaultPreferencesState, - type PreferencesState, -} from '@metamask/preferences-controller'; +import { getDefaultPreferencesState } from '@metamask/preferences-controller'; import assert from 'assert'; import { mockNetwork } from '../../../tests/mock-network'; import { buildInfuraNetworkClientConfiguration } from '../../network-controller/tests/helpers'; +import type { + AllowedActions as AssetsContractAllowedActions, + AllowedEvents as AssetsContractAllowedEvents, +} from './AssetsContractController'; import { AssetsContractController, MISSING_PROVIDER_ERROR, @@ -65,22 +67,25 @@ async function setupAssetContractControllers({ } = {}) { const networkClientConfiguration = { type: NetworkClientType.Infura, - network: 'mainnet', + network: NetworkType.mainnet, infuraProjectId, chainId: BUILT_IN_NETWORKS.mainnet.chainId, ticker: BUILT_IN_NETWORKS.mainnet.ticker, } as const; let provider: Provider; - const messenger: NetworkControllerMessenger = - new ControllerMessenger().getRestricted({ - name: 'NetworkController', - allowedActions: [], - allowedEvents: [], - }); + const controllerMessenger = new ControllerMessenger< + NetworkControllerActions | AssetsContractAllowedActions, + NetworkControllerEvents | AssetsContractAllowedEvents + >(); + const networkMessenger = controllerMessenger.getRestricted({ + name: 'NetworkController', + allowedActions: [], + allowedEvents: [], + }); const networkController = new NetworkController({ infuraProjectId, - messenger, + messenger: networkMessenger, trackMetaMetricsEvent: jest.fn(), }); if (useNetworkControllerProvider) { @@ -88,47 +93,47 @@ async function setupAssetContractControllers({ const selectedNetworkClient = networkController.getSelectedNetworkClient(); assert(selectedNetworkClient, 'No network is selected'); provider = selectedNetworkClient.provider; + + controllerMessenger.unregisterActionHandler( + 'NetworkController:getNetworkClientById', + ); + controllerMessenger.registerActionHandler( + 'NetworkController:getNetworkClientById', + // @ts-expect-error TODO: remove this annotation once the `Eip1193Provider` class is released + (networkClientId: NetworkClientId) => { + return { + ...networkController.getNetworkClientById(networkClientId), + provider, + }; + }, + ); } else { provider = new HttpProvider( `https://mainnet.infura.io/v3/${infuraProjectId}`, ); } - const getNetworkClientById = useNetworkControllerProvider - ? networkController.getNetworkClientById.bind(networkController) - : (networkClientId: NetworkClientId) => - ({ - ...networkController.getNetworkClientById(networkClientId), - provider, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any); - - const preferencesStateChangeListeners: ((state: PreferencesState) => void)[] = - []; + const assetsContractMessenger = controllerMessenger.getRestricted({ + name: 'AssetsContractController', + allowedActions: ['NetworkController:getNetworkClientById'], + allowedEvents: [ + 'PreferencesController:stateChange', + 'NetworkController:networkDidChange', + ], + }); const assetsContract = new AssetsContractController({ chainId: ChainId.mainnet, - onPreferencesStateChange: (listener) => { - preferencesStateChangeListeners.push(listener); - }, - onNetworkDidChange: (listener) => - messenger.subscribe('NetworkController:networkDidChange', listener), - getNetworkClientById, + messenger: assetsContractMessenger, ...options, }); return { - messenger, + messenger: controllerMessenger, network: networkController, assetsContract, provider, networkClientConfiguration, infuraProjectId, - triggerPreferencesStateChange: (state: PreferencesState) => { - for (const listener of preferencesStateChangeListeners) { - listener(state); - } - }, }; } @@ -170,32 +175,41 @@ export { setupAssetContractControllers, mockNetworkWithDefaultChainId }; describe('AssetsContractController', () => { it('should set default config', async () => { const { assetsContract, messenger } = await setupAssetContractControllers(); - expect(assetsContract.config).toStrictEqual({ + expect({ + chainId: assetsContract.chainId, + ipfsGateway: assetsContract.ipfsGateway, + }).toStrictEqual({ chainId: SupportedTokenDetectionNetworks.mainnet, ipfsGateway: IPFS_DEFAULT_GATEWAY_URL, - provider: undefined, }); messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should update the ipfsGateWay config value when this value is changed in the preferences controller', async () => { - const { assetsContract, messenger, triggerPreferencesStateChange } = - await setupAssetContractControllers(); - expect(assetsContract.config).toStrictEqual({ + const { assetsContract, messenger } = await setupAssetContractControllers(); + expect({ + chainId: assetsContract.chainId, + ipfsGateway: assetsContract.ipfsGateway, + }).toStrictEqual({ chainId: SupportedTokenDetectionNetworks.mainnet, ipfsGateway: IPFS_DEFAULT_GATEWAY_URL, - provider: undefined, }); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - ipfsGateway: 'newIPFSGateWay', - }); + messenger.publish( + 'PreferencesController:stateChange', + { + ...getDefaultPreferencesState(), + ipfsGateway: 'newIPFSGateWay', + }, + [], + ); - expect(assetsContract.config).toStrictEqual({ + expect({ + chainId: assetsContract.chainId, + ipfsGateway: assetsContract.ipfsGateway, + }).toStrictEqual({ ipfsGateway: 'newIPFSGateWay', chainId: SupportedTokenDetectionNetworks.mainnet, - provider: undefined, }); messenger.clearEventSubscriptions('NetworkController:networkDidChange'); @@ -211,7 +225,7 @@ describe('AssetsContractController', () => { it('should throw missing provider error when getting ERC-20 token balance when missing provider', async () => { const { assetsContract, messenger } = await setupAssetContractControllers(); - assetsContract.configure({ provider: undefined }); + assetsContract.provider = undefined; await expect( assetsContract.getERC20BalanceOf( ERC20_UNI_ADDRESS, @@ -223,7 +237,7 @@ describe('AssetsContractController', () => { it('should throw missing provider error when getting ERC-20 token decimal when missing provider', async () => { const { assetsContract, messenger } = await setupAssetContractControllers(); - assetsContract.configure({ provider: undefined }); + assetsContract.provider = undefined; await expect( assetsContract.getERC20TokenDecimals(ERC20_UNI_ADDRESS), ).rejects.toThrow(MISSING_PROVIDER_ERROR); @@ -233,7 +247,7 @@ describe('AssetsContractController', () => { it('should get balance of ERC-20 token contract correctly', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -287,7 +301,7 @@ describe('AssetsContractController', () => { it('should get ERC-721 NFT tokenId correctly', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -320,7 +334,7 @@ describe('AssetsContractController', () => { it('should throw missing provider error when getting ERC-721 token standard and details when missing provider', async () => { const { assetsContract, messenger } = await setupAssetContractControllers(); - assetsContract.configure({ provider: undefined }); + assetsContract.provider = undefined; await expect( assetsContract.getTokenStandardAndDetails( ERC20_UNI_ADDRESS, @@ -333,7 +347,7 @@ describe('AssetsContractController', () => { it('should throw contract standard error when getting ERC-20 token standard and details when provided with invalid ERC-20 address', async () => { const { assetsContract, messenger, provider } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; const error = 'Unable to determine contract standard'; await expect( assetsContract.getTokenStandardAndDetails( @@ -347,7 +361,7 @@ describe('AssetsContractController', () => { it('should get ERC-721 token standard and details', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -412,7 +426,7 @@ describe('AssetsContractController', () => { it('should get ERC-1155 token standard and details', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -497,7 +511,7 @@ describe('AssetsContractController', () => { it('should get ERC-20 token standard and details', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -594,7 +608,7 @@ describe('AssetsContractController', () => { it('should get ERC-721 NFT tokenURI correctly', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -643,7 +657,7 @@ describe('AssetsContractController', () => { it('should not throw an error when address given does not support NFT Metadata interface', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; const errorLogSpy = jest .spyOn(console, 'error') .mockImplementationOnce(() => { @@ -701,7 +715,7 @@ describe('AssetsContractController', () => { it('should get ERC-721 NFT name', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -731,7 +745,7 @@ describe('AssetsContractController', () => { it('should get ERC-721 NFT symbol', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -771,7 +785,7 @@ describe('AssetsContractController', () => { it('should get ERC-20 token decimals', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -803,7 +817,7 @@ describe('AssetsContractController', () => { it('should get ERC-20 token name', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -835,7 +849,7 @@ describe('AssetsContractController', () => { it('should get ERC-721 NFT ownership', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -876,7 +890,7 @@ describe('AssetsContractController', () => { it('should get balance of ERC-20 token in a single call on network with token detection support', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -982,7 +996,7 @@ describe('AssetsContractController', () => { useNetworkControllerProvider: true, infuraProjectId, }); - assetsContract.configure({ provider }); + assetsContract.provider = provider; const balancesOnMainnet = await assetsContract.getBalancesInSingleCall( ERC20_SAI_ADDRESS, @@ -1011,7 +1025,7 @@ describe('AssetsContractController', () => { provider, networkClientConfiguration, } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -1082,7 +1096,7 @@ describe('AssetsContractController', () => { it('should throw missing provider error when transferring single ERC-1155 when missing provider', async () => { const { assetsContract, messenger } = await setupAssetContractControllers(); - assetsContract.configure({ provider: undefined }); + assetsContract.provider = undefined; await expect( assetsContract.transferSingleERC1155( ERC1155_ADDRESS, @@ -1098,7 +1112,7 @@ describe('AssetsContractController', () => { it('should throw when ERC1155 function transferSingle is not defined', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -1135,7 +1149,7 @@ describe('AssetsContractController', () => { it('should get the balance of a ERC-1155 NFT for a given address', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -1181,7 +1195,7 @@ describe('AssetsContractController', () => { it('should get the URI of a ERC-1155 NFT', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.configure({ provider }); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ From 27f6f884e68a1ca2a579e372f9a0191482e6b3d3 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 11 Jun 2024 11:40:28 -0400 Subject: [PATCH 05/40] Narrow iindex signature of `SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID` --- .../assets-controllers/src/AssetsContractController.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index fecd96ca2a..28fc5d184a 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -24,7 +24,7 @@ import { ERC721Standard } from './Standards/NftStandards/ERC721/ERC721Standard'; * @param chainId - ChainID of network * @returns Whether the current network supports token detection */ -export const SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID: Record = { +export const SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID = { [SupportedTokenDetectionNetworks.mainnet]: '0xb1f8e55c7f64d203c1400b9d8555d050f94adf39', [SupportedTokenDetectionNetworks.bsc]: @@ -61,7 +61,7 @@ export const SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID: Record = { '0x6aa75276052d96696134252587894ef5ffa520af', [SupportedTokenDetectionNetworks.moonriver]: '0x6aa75276052d96696134252587894ef5ffa520af', -}; +} as const satisfies Record; export const MISSING_PROVIDER_ERROR = 'AssetsContractController failed to set the provider correctly. A provider must be set for this method to be available'; @@ -508,7 +508,10 @@ export class AssetsContractController { ) { const chainId = this.getChainId(networkClientId); const provider = this.getProvider(networkClientId); - if (!(chainId in SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID)) { + if ( + !((id): id is keyof typeof SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID => + id in SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID)(chainId) + ) { // Only fetch balance if contract address exists return {}; } From c6a93233da4b191ba46a4488b69dcbcc029ab1af Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 10 Jul 2024 11:31:12 -0400 Subject: [PATCH 06/40] Define messenger actions corresponding to the public methods of `AssetsContractController` --- packages/assets-controllers/CHANGELOG.md | 4 ++ .../src/AssetsContractController.ts | 38 ++++++++++++++++++- packages/assets-controllers/src/index.ts | 9 ++++- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 869c8c5b06..33f8b2c0e0 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add and export new types `AssetsContractControllerMessenger`, `AssetsContractControllerActions`, `AssetsContractControllerEvents`, `AssetsContractControllerGetERC20StandardAction`, `AssetsContractControllerGetERC721StandardAction`, `AssetsContractControllerGetERC1155StandardAction`, `AssetsContractControllerGetERC20BalanceOfAction`, `AssetsContractControllerGetERC20TokenDecimalsAction`, `AssetsContractControllerGetERC20TokenNameAction`, `AssetsContractControllerGetERC721NftTokenIdAction`, `AssetsContractControllerGetERC721TokenURIAction`, `AssetsContractControllerGetERC721AssetNameAction`, `AssetsContractControllerGetERC721AssetSymbolAction`, `AssetsContractControllerGetERC721OwnerOfAction`, `AssetsContractControllerGetERC1155TokenURIAction`, `AssetsContractControllerGetERC1155BalanceOfAction`, `AssetsContractControllerTransferSingleERC1155Action`, `AssetsContractControllerGetTokenStandardAndDetailsAction`, `AssetsContractControllerGetBalancesInSingleCallAction` [#4397](https://github.com/MetaMask/core/pull/4397) + ## [35.0.0] ### Changed diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 28fc5d184a..68e970450f 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -78,6 +78,40 @@ export type BalanceMap = { export const name = 'AssetsContractController'; +export type AssetsContractControllerGetERC20StandardAction = { + type: `${typeof name}:getERC20Standard`; + handler: AssetsContractController['getERC20Standard']; +}; + +export type AssetsContractControllerGetERC721StandardAction = { + type: `${typeof name}:getERC721Standard`; + handler: AssetsContractController['getERC721Standard']; +}; + +export type AssetsContractControllerGetERC1155StandardAction = { + type: `${typeof name}:getERC1155Standard`; + handler: AssetsContractController['getERC1155Standard']; +}; + +export type AssetsContractControllerGetTokenStandardAndDetailsAction = { + type: `${typeof name}:getTokenStandardAndDetails`; + handler: AssetsContractController['getTokenStandardAndDetails']; +}; + +export type AssetsContractControllerGetBalancesInSingleCallAction = { + type: `${typeof name}:getBalancesInSingleCall`; + handler: AssetsContractController['getBalancesInSingleCall']; +}; + +export type AssetsContractControllerActions = + | AssetsContractControllerGetERC20StandardAction + | AssetsContractControllerGetERC721StandardAction + | AssetsContractControllerGetERC1155StandardAction + | AssetsContractControllerGetTokenStandardAndDetailsAction + | AssetsContractControllerGetBalancesInSingleCallAction; + +export type AssetsContractControllerEvents = never; + export type AllowedActions = NetworkControllerGetNetworkClientByIdAction; export type AllowedEvents = @@ -86,8 +120,8 @@ export type AllowedEvents = export type AssetsContractControllerMessenger = RestrictedControllerMessenger< typeof name, - AllowedActions, - AllowedEvents, + AssetsContractControllerActions | AllowedActions, + AssetsContractControllerEvents | AllowedEvents, AllowedActions['type'], AllowedEvents['type'] >; diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index 1888365935..b3b6d11066 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -9,8 +9,15 @@ export type { } from './AccountTrackerController'; export { AccountTrackerController } from './AccountTrackerController'; export type { - BalanceMap, + AssetsContractControllerActions, + AssetsContractControllerEvents, + AssetsContractControllerGetERC20StandardAction, + AssetsContractControllerGetERC721StandardAction, + AssetsContractControllerGetERC1155StandardAction, + AssetsContractControllerGetTokenStandardAndDetailsAction, + AssetsContractControllerGetBalancesInSingleCallAction, AssetsContractControllerMessenger, + BalanceMap, } from './AssetsContractController'; export { SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID, From 35a3bf99f8e2122b346ba8e0fcd9805f6bf196f3 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 13 Jun 2024 12:40:32 -0400 Subject: [PATCH 07/40] Convert getters for `provider`, `chainId` into private methods --- .../src/AssetsContractController.ts | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 68e970450f..def7f0d52d 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -152,10 +152,14 @@ export class AssetsContractController { messenger: AssetsContractControllerMessenger; chainId: Hex; }) { - this.chainId = initialChainId; + this.messagingSystem = messenger; this.#provider = undefined; this.ipfsGateway = IPFS_DEFAULT_GATEWAY_URL; - this.messagingSystem = messenger; + this.chainId = initialChainId; + + this.messagingSystem.registerActionHandler('AssetsContractController:getERCStandard', this.getERCStandard.bind(this)); + this.messagingSystem.registerActionHandler('AssetsContractController:getTokenStandardAndDetails', this.getTokenStandardAndDetails.bind(this)); + this.messagingSystem.registerActionHandler('AssetsContractController:getBalancesInSingleCall', this.getBalancesInSingleCall.bind(this)); this.messagingSystem.subscribe( `PreferencesController:stateChange`, @@ -176,6 +180,9 @@ export class AssetsContractController { if (this.chainId !== chainId) { this.chainId = chainId; + + // @ts-expect-error TODO: remove this annotation once the `Eip1193Provider` class is released + this.#provider = this.#getCorrectProvider(); } }, ); @@ -202,7 +209,7 @@ export class AssetsContractController { * @param networkClientId - Network Client ID. * @returns Web3Provider instance. */ - getProvider(networkClientId?: NetworkClientId): Web3Provider { + #getCorrectProvider(networkClientId?: NetworkClientId): Web3Provider { const provider = networkClientId ? this.messagingSystem.call( `NetworkController:getNetworkClientById`, @@ -223,7 +230,7 @@ export class AssetsContractController { * @param networkClientId - Network Client ID used to get the provider. * @returns Hex chain ID. */ - getChainId(networkClientId?: NetworkClientId): Hex { + #getCorrectChainId(networkClientId?: NetworkClientId): Hex { return networkClientId ? this.messagingSystem.call( `NetworkController:getNetworkClientById`, @@ -239,7 +246,7 @@ export class AssetsContractController { * @returns ERC20Standard instance. */ getERC20Standard(networkClientId?: NetworkClientId): ERC20Standard { - const provider = this.getProvider(networkClientId); + const provider = this.#getCorrectProvider(networkClientId); return new ERC20Standard(provider); } @@ -250,7 +257,7 @@ export class AssetsContractController { * @returns ERC721Standard instance. */ getERC721Standard(networkClientId?: NetworkClientId): ERC721Standard { - const provider = this.getProvider(networkClientId); + const provider = this.#getCorrectProvider(networkClientId); return new ERC721Standard(provider); } @@ -261,7 +268,7 @@ export class AssetsContractController { * @returns ERC1155Standard instance. */ getERC1155Standard(networkClientId?: NetworkClientId): ERC1155Standard { - const provider = this.getProvider(networkClientId); + const provider = this.#getCorrectProvider(networkClientId); return new ERC1155Standard(provider); } @@ -354,7 +361,7 @@ export class AssetsContractController { balance?: BN | undefined; }> { // Asserts provider is available - this.getProvider(networkClientId); + this.#getCorrectProvider(networkClientId); // ERC721 try { @@ -540,8 +547,8 @@ export class AssetsContractController { tokensToDetect: string[], networkClientId?: NetworkClientId, ) { - const chainId = this.getChainId(networkClientId); - const provider = this.getProvider(networkClientId); + const chainId = this.#getCorrectChainId(networkClientId); + const provider = this.#getCorrectProvider(networkClientId); if ( !((id): id is keyof typeof SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID => id in SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID)(chainId) From 4c37cfe50e7d8c06700a23998f832ab939cb1170 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 13 Jun 2024 12:40:44 -0400 Subject: [PATCH 08/40] Register action handlers --- .../src/AssetsContractController.ts | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index def7f0d52d..4f20d257ec 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -104,10 +104,10 @@ export type AssetsContractControllerGetBalancesInSingleCallAction = { }; export type AssetsContractControllerActions = - | AssetsContractControllerGetERC20StandardAction - | AssetsContractControllerGetERC721StandardAction - | AssetsContractControllerGetERC1155StandardAction - | AssetsContractControllerGetTokenStandardAndDetailsAction + | AssetsContractControllerGetERC20StandardAction + | AssetsContractControllerGetERC721StandardAction + | AssetsContractControllerGetERC1155StandardAction + | AssetsContractControllerGetTokenStandardAndDetailsAction | AssetsContractControllerGetBalancesInSingleCallAction; export type AssetsContractControllerEvents = never; @@ -157,9 +157,26 @@ export class AssetsContractController { this.ipfsGateway = IPFS_DEFAULT_GATEWAY_URL; this.chainId = initialChainId; - this.messagingSystem.registerActionHandler('AssetsContractController:getERCStandard', this.getERCStandard.bind(this)); - this.messagingSystem.registerActionHandler('AssetsContractController:getTokenStandardAndDetails', this.getTokenStandardAndDetails.bind(this)); - this.messagingSystem.registerActionHandler('AssetsContractController:getBalancesInSingleCall', this.getBalancesInSingleCall.bind(this)); + this.messagingSystem.registerActionHandler( + 'AssetsContractController:getERC20Standard', + this.getERC20Standard.bind(this), + ); + this.messagingSystem.registerActionHandler( + 'AssetsContractController:getERC721Standard', + this.getERC721Standard.bind(this), + ); + this.messagingSystem.registerActionHandler( + 'AssetsContractController:getERC1155Standard', + this.getERC1155Standard.bind(this), + ); + this.messagingSystem.registerActionHandler( + 'AssetsContractController:getTokenStandardAndDetails', + this.getTokenStandardAndDetails.bind(this), + ); + this.messagingSystem.registerActionHandler( + 'AssetsContractController:getBalancesInSingleCall', + this.getBalancesInSingleCall.bind(this), + ); this.messagingSystem.subscribe( `PreferencesController:stateChange`, From 79a29c3393fe718dbeb8cdfaae88dbb1dc1b0ef4 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 10 Jul 2024 13:49:45 -0400 Subject: [PATCH 09/40] Replace AssetsContractController constructor options with messenger actions in downstream controllers --- .../assets-controllers/src/NftController.ts | 89 ++++++------------- .../src/TokenBalancesController.ts | 35 +++----- 2 files changed, 41 insertions(+), 83 deletions(-) diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index c305eeece6..bb57a7337d 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -44,7 +44,11 @@ import { Mutex } from 'async-mutex'; import BN from 'bn.js'; import { v4 as random } from 'uuid'; -import type { AssetsContractController } from './AssetsContractController'; +import type { + AssetsContractControllerGetERC1155StandardAction, + AssetsContractControllerGetERC20StandardAction, + AssetsContractControllerGetERC721StandardAction, +} from './AssetsContractController'; import { compareNftMetadata, getFormattedIpfsUrl, @@ -230,7 +234,10 @@ export type AllowedActions = | AddApprovalRequest | AccountsControllerGetAccountAction | AccountsControllerGetSelectedAccountAction - | NetworkControllerGetNetworkClientByIdAction; + | NetworkControllerGetNetworkClientByIdAction + | AssetsContractControllerGetERC20StandardAction + | AssetsContractControllerGetERC721StandardAction + | AssetsContractControllerGetERC1155StandardAction; export type AllowedEvents = | PreferencesControllerStateChangeEvent @@ -288,18 +295,6 @@ export class NftController extends BaseController< #isIpfsGatewayEnabled: boolean; - 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 #onNftAdded?: (data: { address: string; symbol: string | undefined; @@ -317,12 +312,6 @@ export class NftController extends BaseController< * @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. - * @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.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. @@ -334,12 +323,6 @@ export class NftController extends BaseController< openSeaEnabled = false, useIpfsSubdomains = true, isIpfsGatewayEnabled = true, - getERC721AssetName, - getERC721AssetSymbol, - getERC721TokenURI, - getERC721OwnerOf, - getERC1155BalanceOf, - getERC1155TokenURI, onNftAdded, messenger, state = {}, @@ -349,12 +332,6 @@ export class NftController extends BaseController< openSeaEnabled?: boolean; useIpfsSubdomains?: boolean; isIpfsGatewayEnabled?: boolean; - getERC721AssetName: AssetsContractController['getERC721AssetName']; - getERC721AssetSymbol: AssetsContractController['getERC721AssetSymbol']; - getERC721TokenURI: AssetsContractController['getERC721TokenURI']; - getERC721OwnerOf: AssetsContractController['getERC721OwnerOf']; - getERC1155BalanceOf: AssetsContractController['getERC1155BalanceOf']; - getERC1155TokenURI: AssetsContractController['getERC1155TokenURI']; onNftAdded?: (data: { address: string; symbol: string | undefined; @@ -383,13 +360,6 @@ export class NftController extends BaseController< this.#openSeaEnabled = openSeaEnabled; this.#useIpfsSubdomains = useIpfsSubdomains; this.#isIpfsGatewayEnabled = isIpfsGatewayEnabled; - - this.#getERC721AssetName = getERC721AssetName; - this.#getERC721AssetSymbol = getERC721AssetSymbol; - this.#getERC721TokenURI = getERC721TokenURI; - this.#getERC721OwnerOf = getERC721OwnerOf; - this.#getERC1155BalanceOf = getERC1155BalanceOf; - this.#getERC1155TokenURI = getERC1155TokenURI; this.#onNftAdded = onNftAdded; this.messagingSystem.subscribe( @@ -735,11 +705,9 @@ export class NftController extends BaseController< ): Promise<[string, string]> { // try ERC721 uri try { - const uri = await this.#getERC721TokenURI( - contractAddress, - tokenId, - networkClientId, - ); + const uri = await this.messagingSystem + .call('AssetsContractController:getERC721Standard', networkClientId) + .getTokenURI(contractAddress, tokenId); return [uri, ERC721]; } catch { // Ignore error @@ -747,11 +715,9 @@ export class NftController extends BaseController< // try ERC1155 uri try { - const tokenURI = await this.#getERC1155TokenURI( - contractAddress, - tokenId, - networkClientId, - ); + const tokenURI = await this.messagingSystem + .call('AssetsContractController:getERC1155Standard', networkClientId) + .getTokenURI(contractAddress, tokenId); /** * According to EIP1155 the URI value allows for ID substitution @@ -832,8 +798,12 @@ export class NftController extends BaseController< Pick > { const [name, symbol] = await Promise.all([ - this.#getERC721AssetName(contractAddress, networkClientId), - this.#getERC721AssetSymbol(contractAddress, networkClientId), + this.messagingSystem + .call('AssetsContractController:getERC721Standard', networkClientId) + .getAssetName(contractAddress), + this.messagingSystem + .call('AssetsContractController:getERC721Standard', networkClientId) + .getAssetSymbol(contractAddress), ]); return { @@ -1413,11 +1383,9 @@ export class NftController extends BaseController< ): Promise { // Checks the ownership for ERC-721. try { - const owner = await this.#getERC721OwnerOf( - nftAddress, - tokenId, - networkClientId, - ); + const owner = await this.messagingSystem + .call('AssetsContractController:getERC721Standard', networkClientId) + .getOwnerOf(nftAddress, tokenId); return ownerAddress.toLowerCase() === owner.toLowerCase(); // eslint-disable-next-line no-empty } catch { @@ -1426,12 +1394,9 @@ export class NftController extends BaseController< // Checks the ownership for ERC-1155. try { - const balance = await this.#getERC1155BalanceOf( - ownerAddress, - nftAddress, - tokenId, - networkClientId, - ); + const balance = await this.messagingSystem + .call('AssetsContractController:getERC1155Standard', networkClientId) + .getBalanceOf(ownerAddress, nftAddress, tokenId); return !balance.isZero(); // eslint-disable-next-line no-empty } catch { diff --git a/packages/assets-controllers/src/TokenBalancesController.ts b/packages/assets-controllers/src/TokenBalancesController.ts index a1b58a4340..b7c57325df 100644 --- a/packages/assets-controllers/src/TokenBalancesController.ts +++ b/packages/assets-controllers/src/TokenBalancesController.ts @@ -1,13 +1,13 @@ -import { type AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller'; -import { - type RestrictedControllerMessenger, - type ControllerGetStateAction, - type ControllerStateChangeEvent, - BaseController, +import type { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller'; +import type { + RestrictedControllerMessenger, + ControllerGetStateAction, + ControllerStateChangeEvent, } from '@metamask/base-controller'; +import { BaseController } from '@metamask/base-controller'; import { safelyExecute, toHex } from '@metamask/controller-utils'; -import type { AssetsContractController } from './AssetsContractController'; +import type { AssetsContractControllerGetERC20StandardAction } from './AssetsContractController'; import type { Token } from './TokenRatesController'; import type { TokensControllerStateChangeEvent } from './TokensController'; @@ -24,13 +24,11 @@ const metadata = { * @property interval - Polling interval used to fetch new token balances. * @property tokens - List of tokens to track balances for. * @property disabled - If set to true, all tracked tokens contract balances updates are blocked. - * @property getERC20BalanceOf - Gets the balance of the given account at the given contract address. */ type TokenBalancesControllerOptions = { interval?: number; tokens?: Token[]; disabled?: boolean; - getERC20BalanceOf: AssetsContractController['getERC20BalanceOf']; messenger: TokenBalancesControllerMessenger; state?: Partial; }; @@ -56,8 +54,10 @@ export type TokenBalancesControllerGetStateAction = ControllerGetStateAction< export type TokenBalancesControllerActions = TokenBalancesControllerGetStateAction; -export type AllowedActions = AccountsControllerGetSelectedAccountAction; - +export type AllowedActions = + | AccountsControllerGetSelectedAccountAction + | AssetsContractControllerGetERC20StandardAction; + export type TokenBalancesControllerStateChangeEvent = ControllerStateChangeEvent< typeof controllerName, @@ -99,8 +99,6 @@ export class TokenBalancesController extends BaseController< > { #handle?: ReturnType; - #getERC20BalanceOf: AssetsContractController['getERC20BalanceOf']; - #interval: number; #tokens: Token[]; @@ -114,7 +112,6 @@ export class TokenBalancesController extends BaseController< * @param options.interval - Polling interval used to fetch new token balances. * @param options.tokens - List of tokens to track balances for. * @param options.disabled - If set to true, all tracked tokens contract balances updates are blocked. - * @param options.getERC20BalanceOf - Gets the balance of the given account at the given contract address. * @param options.state - Initial state to set on this controller. * @param options.messenger - The controller restricted messenger. */ @@ -122,7 +119,6 @@ export class TokenBalancesController extends BaseController< interval = DEFAULT_INTERVAL, tokens = [], disabled = false, - getERC20BalanceOf, messenger, state = {}, }: TokenBalancesControllerOptions) { @@ -150,8 +146,6 @@ export class TokenBalancesController extends BaseController< }, ); - this.#getERC20BalanceOf = getERC20BalanceOf; - // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/no-floating-promises this.poll(); @@ -209,10 +203,9 @@ export class TokenBalancesController extends BaseController< for (const token of this.#tokens) { const { address } = token; try { - const balance = await this.#getERC20BalanceOf( - address, - selectedInternalAccount.address, - ); + const balance = await this.messagingSystem + .call('AssetsContractController:getERC20Standard') + .getBalanceOf(address, selectedInternalAccount.address); newContractBalances[address] = toHex(balance); token.hasBalanceError = false; } catch (error) { From 1315c887735233848560a2ee1b9dc94d9abe6a37 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 13 Jun 2024 15:39:53 -0400 Subject: [PATCH 10/40] Linter fix --- packages/assets-controllers/src/AssetsContractController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 4f20d257ec..665983d116 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -103,7 +103,7 @@ export type AssetsContractControllerGetBalancesInSingleCallAction = { handler: AssetsContractController['getBalancesInSingleCall']; }; -export type AssetsContractControllerActions = +export type AssetsContractControllerActions = | AssetsContractControllerGetERC20StandardAction | AssetsContractControllerGetERC721StandardAction | AssetsContractControllerGetERC1155StandardAction From 83271e7dda47bf040cc0ccbdc96fddaf8704250a Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 10 Jul 2024 11:31:38 -0400 Subject: [PATCH 11/40] Convert class methods into messenger actions --- .../src/AssetsContractController.ts | 89 +++++++++++++------ packages/assets-controllers/src/index.ts | 11 +++ 2 files changed, 73 insertions(+), 27 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 665983d116..c655e3421a 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -1,6 +1,9 @@ import { Contract } from '@ethersproject/contracts'; import { Web3Provider } from '@ethersproject/providers'; -import type { RestrictedControllerMessenger } from '@metamask/base-controller'; +import type { + ActionConstraint, + RestrictedControllerMessenger, +} from '@metamask/base-controller'; import { IPFS_DEFAULT_GATEWAY_URL } from '@metamask/controller-utils'; import type { NetworkClientId, @@ -76,39 +79,71 @@ export type BalanceMap = { [tokenAddress: string]: BN; }; -export const name = 'AssetsContractController'; +const name = 'AssetsContractController'; -export type AssetsContractControllerGetERC20StandardAction = { - type: `${typeof name}:getERC20Standard`; - handler: AssetsContractController['getERC20Standard']; +// TODO: Convert into generic type that takes controller type as input and move to base-controller +type AssetsContractControllerActionsMap = { + [ClassMethod in keyof AssetsContractController as AssetsContractController[ClassMethod] extends ActionConstraint['handler'] + ? ClassMethod + : never]: { + type: `${typeof name}:${ClassMethod}`; + handler: AssetsContractController[ClassMethod]; + }; }; -export type AssetsContractControllerGetERC721StandardAction = { - type: `${typeof name}:getERC721Standard`; - handler: AssetsContractController['getERC721Standard']; -}; +type AssetsContractControllerMethodName = + keyof AssetsContractControllerActionsMap; -export type AssetsContractControllerGetERC1155StandardAction = { - type: `${typeof name}:getERC1155Standard`; - handler: AssetsContractController['getERC1155Standard']; -}; +export type AssetsContractControllerActions = + AssetsContractControllerActionsMap[AssetsContractControllerMethodName]; -export type AssetsContractControllerGetTokenStandardAndDetailsAction = { - type: `${typeof name}:getTokenStandardAndDetails`; - handler: AssetsContractController['getTokenStandardAndDetails']; -}; +export type AssetsContractControllerGetERC20StandardAction = + AssetsContractControllerActionsMap['getERC20Standard']; -export type AssetsContractControllerGetBalancesInSingleCallAction = { - type: `${typeof name}:getBalancesInSingleCall`; - handler: AssetsContractController['getBalancesInSingleCall']; -}; +export type AssetsContractControllerGetERC721StandardAction = + AssetsContractControllerActionsMap['getERC721Standard']; -export type AssetsContractControllerActions = - | AssetsContractControllerGetERC20StandardAction - | AssetsContractControllerGetERC721StandardAction - | AssetsContractControllerGetERC1155StandardAction - | AssetsContractControllerGetTokenStandardAndDetailsAction - | AssetsContractControllerGetBalancesInSingleCallAction; +export type AssetsContractControllerGetERC1155StandardAction = + AssetsContractControllerActionsMap['getERC1155Standard']; + +export type AssetsContractControllerGetERC20BalanceOfAction = + AssetsContractControllerActionsMap['getERC20BalanceOf']; + +export type AssetsContractControllerGetERC20TokenDecimalsAction = + AssetsContractControllerActionsMap['getERC20TokenDecimals']; + +export type AssetsContractControllerGetERC20TokenNameAction = + AssetsContractControllerActionsMap['getERC20TokenName']; + +export type AssetsContractControllerGetERC721NftTokenIdAction = + AssetsContractControllerActionsMap['getERC721NftTokenId']; + +export type AssetsContractControllerGetERC721TokenURIAction = + AssetsContractControllerActionsMap['getERC721TokenURI']; + +export type AssetsContractControllerGetERC721AssetNameAction = + AssetsContractControllerActionsMap['getERC721AssetName']; + +export type AssetsContractControllerGetERC721AssetSymbolAction = + AssetsContractControllerActionsMap['getERC721AssetSymbol']; + +export type AssetsContractControllerGetERC721OwnerOfAction = + AssetsContractControllerActionsMap['getERC721OwnerOf']; + +export type AssetsContractControllerGetERC1155TokenURIAction = + AssetsContractControllerActionsMap['getERC1155TokenURI']; + +export type AssetsContractControllerGetERC1155BalanceOfAction = + AssetsContractControllerActionsMap['getERC1155BalanceOf']; + +export type AssetsContractControllerTransferSingleERC1155Action = + AssetsContractControllerActionsMap['transferSingleERC1155']; + +export type AssetsContractControllerGetTokenStandardAndDetailsAction = + AssetsContractControllerActionsMap['getTokenStandardAndDetails']; + +export type AssetsContractControllerGetBalancesInSingleCallAction = + AssetsContractControllerActionsMap['getBalancesInSingleCall']; export type AssetsContractControllerEvents = never; diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index b3b6d11066..a233bf096d 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -14,6 +14,17 @@ export type { AssetsContractControllerGetERC20StandardAction, AssetsContractControllerGetERC721StandardAction, AssetsContractControllerGetERC1155StandardAction, + AssetsContractControllerGetERC20BalanceOfAction, + AssetsContractControllerGetERC20TokenDecimalsAction, + AssetsContractControllerGetERC20TokenNameAction, + AssetsContractControllerGetERC721NftTokenIdAction, + AssetsContractControllerGetERC721TokenURIAction, + AssetsContractControllerGetERC721AssetNameAction, + AssetsContractControllerGetERC721AssetSymbolAction, + AssetsContractControllerGetERC721OwnerOfAction, + AssetsContractControllerGetERC1155TokenURIAction, + AssetsContractControllerGetERC1155BalanceOfAction, + AssetsContractControllerTransferSingleERC1155Action, AssetsContractControllerGetTokenStandardAndDetailsAction, AssetsContractControllerGetBalancesInSingleCallAction, AssetsContractControllerMessenger, From 123fe80be9ac2f28f23033dcb6f735fc652b322c Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 26 Jun 2024 17:16:15 -0400 Subject: [PATCH 12/40] Register class methods as action handlers --- .../src/AssetsContractController.ts | 50 +++++++++++-------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index c655e3421a..126d3d17aa 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -192,27 +192,36 @@ export class AssetsContractController { this.ipfsGateway = IPFS_DEFAULT_GATEWAY_URL; this.chainId = initialChainId; - this.messagingSystem.registerActionHandler( - 'AssetsContractController:getERC20Standard', - this.getERC20Standard.bind(this), - ); - this.messagingSystem.registerActionHandler( - 'AssetsContractController:getERC721Standard', - this.getERC721Standard.bind(this), - ); - this.messagingSystem.registerActionHandler( - 'AssetsContractController:getERC1155Standard', - this.getERC1155Standard.bind(this), - ); - this.messagingSystem.registerActionHandler( - 'AssetsContractController:getTokenStandardAndDetails', - this.getTokenStandardAndDetails.bind(this), - ); - this.messagingSystem.registerActionHandler( - 'AssetsContractController:getBalancesInSingleCall', - this.getBalancesInSingleCall.bind(this), - ); + this.#registerActionHandlers(); + this.#registerEventSubscriptions(); + } + // TODO: Expand into base-controller utility function that batch registers action handlers. + #registerActionHandlers() { + for (const method of Object.getOwnPropertyNames( + Object.getPrototypeOf(this), + ) as (keyof this)[]) { + if ( + ((key: keyof this): key is AssetsContractControllerMethodName => + ![ + 'constructor', + 'messagingSystem', + 'provider', + 'ipfsGateway', + 'chainId', + ].find((e) => e === key) && typeof this[key] === 'function')(method) + ) { + this.messagingSystem.registerActionHandler( + `${name}:${method}`, + // TODO: Write a for-loop function that iterates over an input union type in tandem with the input array. + // @ts-expect-error Both assigned argument and assignee parameter are using the entire union type for `method` + this[method].bind(this), + ); + } + } + } + + #registerEventSubscriptions() { this.messagingSystem.subscribe( `PreferencesController:stateChange`, ({ ipfsGateway }) => { @@ -232,7 +241,6 @@ export class AssetsContractController { if (this.chainId !== chainId) { this.chainId = chainId; - // @ts-expect-error TODO: remove this annotation once the `Eip1193Provider` class is released this.#provider = this.#getCorrectProvider(); } From ee3a9236c0dc87b25e65611febb89c53f7b22d8e Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 26 Jun 2024 17:18:26 -0400 Subject: [PATCH 13/40] Add assets-contract actions as allowed actions to downstream controllers --- .../assets-controllers/src/NftController.ts | 71 ++++++++++++------- .../src/TokenBalancesController.ts | 14 ++-- 2 files changed, 55 insertions(+), 30 deletions(-) diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index bb57a7337d..7ea8328567 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -45,9 +45,12 @@ import BN from 'bn.js'; import { v4 as random } from 'uuid'; import type { - AssetsContractControllerGetERC1155StandardAction, - AssetsContractControllerGetERC20StandardAction, - AssetsContractControllerGetERC721StandardAction, + AssetsContractControllerGetERC1155BalanceOfAction, + AssetsContractControllerGetERC1155TokenURIAction, + AssetsContractControllerGetERC721AssetNameAction, + AssetsContractControllerGetERC721AssetSymbolAction, + AssetsContractControllerGetERC721OwnerOfAction, + AssetsContractControllerGetERC721TokenURIAction, } from './AssetsContractController'; import { compareNftMetadata, @@ -235,9 +238,12 @@ export type AllowedActions = | AccountsControllerGetAccountAction | AccountsControllerGetSelectedAccountAction | NetworkControllerGetNetworkClientByIdAction - | AssetsContractControllerGetERC20StandardAction - | AssetsContractControllerGetERC721StandardAction - | AssetsContractControllerGetERC1155StandardAction; + | AssetsContractControllerGetERC721AssetNameAction + | AssetsContractControllerGetERC721AssetSymbolAction + | AssetsContractControllerGetERC721TokenURIAction + | AssetsContractControllerGetERC721OwnerOfAction + | AssetsContractControllerGetERC1155BalanceOfAction + | AssetsContractControllerGetERC1155TokenURIAction; export type AllowedEvents = | PreferencesControllerStateChangeEvent @@ -705,9 +711,12 @@ export class NftController extends BaseController< ): Promise<[string, string]> { // try ERC721 uri try { - const uri = await this.messagingSystem - .call('AssetsContractController:getERC721Standard', networkClientId) - .getTokenURI(contractAddress, tokenId); + const uri = await this.messagingSystem.call( + 'AssetsContractController:getERC721TokenURI', + contractAddress, + tokenId, + networkClientId, + ); return [uri, ERC721]; } catch { // Ignore error @@ -715,9 +724,12 @@ export class NftController extends BaseController< // try ERC1155 uri try { - const tokenURI = await this.messagingSystem - .call('AssetsContractController:getERC1155Standard', networkClientId) - .getTokenURI(contractAddress, tokenId); + const tokenURI = await this.messagingSystem.call( + 'AssetsContractController:getERC1155TokenURI', + contractAddress, + tokenId, + networkClientId, + ); /** * According to EIP1155 the URI value allows for ID substitution @@ -798,12 +810,16 @@ export class NftController extends BaseController< Pick > { const [name, symbol] = await Promise.all([ - this.messagingSystem - .call('AssetsContractController:getERC721Standard', networkClientId) - .getAssetName(contractAddress), - this.messagingSystem - .call('AssetsContractController:getERC721Standard', networkClientId) - .getAssetSymbol(contractAddress), + this.messagingSystem.call( + 'AssetsContractController:getERC721AssetName', + contractAddress, + networkClientId, + ), + this.messagingSystem.call( + 'AssetsContractController:getERC721AssetSymbol', + contractAddress, + networkClientId, + ), ]); return { @@ -1383,9 +1399,12 @@ export class NftController extends BaseController< ): Promise { // Checks the ownership for ERC-721. try { - const owner = await this.messagingSystem - .call('AssetsContractController:getERC721Standard', networkClientId) - .getOwnerOf(nftAddress, tokenId); + const owner = await this.messagingSystem.call( + 'AssetsContractController:getERC721OwnerOf', + nftAddress, + tokenId, + networkClientId, + ); return ownerAddress.toLowerCase() === owner.toLowerCase(); // eslint-disable-next-line no-empty } catch { @@ -1394,9 +1413,13 @@ export class NftController extends BaseController< // Checks the ownership for ERC-1155. try { - const balance = await this.messagingSystem - .call('AssetsContractController:getERC1155Standard', networkClientId) - .getBalanceOf(ownerAddress, nftAddress, tokenId); + const balance = await this.messagingSystem.call( + 'AssetsContractController:getERC1155BalanceOf', + ownerAddress, + nftAddress, + tokenId, + networkClientId, + ); return !balance.isZero(); // eslint-disable-next-line no-empty } catch { diff --git a/packages/assets-controllers/src/TokenBalancesController.ts b/packages/assets-controllers/src/TokenBalancesController.ts index b7c57325df..e80db70d5f 100644 --- a/packages/assets-controllers/src/TokenBalancesController.ts +++ b/packages/assets-controllers/src/TokenBalancesController.ts @@ -7,7 +7,7 @@ import type { import { BaseController } from '@metamask/base-controller'; import { safelyExecute, toHex } from '@metamask/controller-utils'; -import type { AssetsContractControllerGetERC20StandardAction } from './AssetsContractController'; +import type { AssetsContractControllerGetERC20BalanceOfAction } from './AssetsContractController'; import type { Token } from './TokenRatesController'; import type { TokensControllerStateChangeEvent } from './TokensController'; @@ -56,8 +56,8 @@ export type TokenBalancesControllerActions = export type AllowedActions = | AccountsControllerGetSelectedAccountAction - | AssetsContractControllerGetERC20StandardAction; - + | AssetsContractControllerGetERC20BalanceOfAction; + export type TokenBalancesControllerStateChangeEvent = ControllerStateChangeEvent< typeof controllerName, @@ -203,9 +203,11 @@ export class TokenBalancesController extends BaseController< for (const token of this.#tokens) { const { address } = token; try { - const balance = await this.messagingSystem - .call('AssetsContractController:getERC20Standard') - .getBalanceOf(address, selectedInternalAccount.address); + const balance = await this.messagingSystem.call( + 'AssetsContractController:getERC20BalanceOf', + address, + selectedInternalAccount.address, + ); newContractBalances[address] = toHex(balance); token.hasBalanceError = false; } catch (error) { From 10a4678f1b023268c6feccec7dec2b0e0ca03d09 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 27 Jun 2024 17:01:12 -0400 Subject: [PATCH 14/40] [assets-contract] Adjust downstream tests to use messenger actions instead of class method callbacks --- .../src/AssetsContractController.test.ts | 123 +++- ...tractControllerWithNetworkClientId.test.ts | 126 ++-- .../src/NftController.test.ts | 664 +++++++++--------- .../src/TokenBalancesController.test.ts | 21 +- 4 files changed, 506 insertions(+), 428 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.test.ts b/packages/assets-controllers/src/AssetsContractController.test.ts index f1c2319d5d..28668b5354 100644 --- a/packages/assets-controllers/src/AssetsContractController.test.ts +++ b/packages/assets-controllers/src/AssetsContractController.test.ts @@ -24,6 +24,8 @@ import assert from 'assert'; import { mockNetwork } from '../../../tests/mock-network'; import { buildInfuraNetworkClientConfiguration } from '../../network-controller/tests/helpers'; import type { + AssetsContractControllerActions, + AssetsContractControllerEvents, AllowedActions as AssetsContractAllowedActions, AllowedEvents as AssetsContractAllowedEvents, } from './AssetsContractController'; @@ -61,7 +63,9 @@ async function setupAssetContractControllers({ useNetworkControllerProvider, infuraProjectId = '341eacb578dd44a1a049cbc5f6fd4035', }: { - options?: Partial[0]>; + options?: Partial< + Omit[0], 'messenger'> + >; useNetworkControllerProvider?: boolean; infuraProjectId?: string; } = {}) { @@ -75,8 +79,12 @@ async function setupAssetContractControllers({ let provider: Provider; const controllerMessenger = new ControllerMessenger< - NetworkControllerActions | AssetsContractAllowedActions, - NetworkControllerEvents | AssetsContractAllowedEvents + | NetworkControllerActions + | AssetsContractControllerActions + | AssetsContractAllowedActions, + | NetworkControllerEvents + | AssetsContractControllerEvents + | AssetsContractAllowedEvents >(); const networkMessenger = controllerMessenger.getRestricted({ name: 'NetworkController', @@ -227,7 +235,8 @@ describe('AssetsContractController', () => { const { assetsContract, messenger } = await setupAssetContractControllers(); assetsContract.provider = undefined; await expect( - assetsContract.getERC20BalanceOf( + messenger.call( + `AssetsContractController:getERC20BalanceOf`, ERC20_UNI_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, ), @@ -239,7 +248,10 @@ describe('AssetsContractController', () => { const { assetsContract, messenger } = await setupAssetContractControllers(); assetsContract.provider = undefined; await expect( - assetsContract.getERC20TokenDecimals(ERC20_UNI_ADDRESS), + messenger.call( + `AssetsContractController:getERC20TokenDecimals`, + ERC20_UNI_ADDRESS, + ), ).rejects.toThrow(MISSING_PROVIDER_ERROR); messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); @@ -285,11 +297,13 @@ describe('AssetsContractController', () => { }, ], }); - const UNIBalance = await assetsContract.getERC20BalanceOf( + const UNIBalance = await messenger.call( + `AssetsContractController:getERC20BalanceOf`, ERC20_UNI_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, ); - const UNINoBalance = await assetsContract.getERC20BalanceOf( + const UNINoBalance = await messenger.call( + `AssetsContractController:getERC20BalanceOf`, ERC20_UNI_ADDRESS, '0x202637dAAEfbd7f131f90338a4A6c69F6Cd5CE91', ); @@ -323,7 +337,8 @@ describe('AssetsContractController', () => { }, ], }); - const tokenId = await assetsContract.getERC721NftTokenId( + const tokenId = await messenger.call( + `AssetsContractController:getERC721NftTokenId`, ERC721_GODS_ADDRESS, '0x9a90bd8d1149a88b42a99cf62215ad955d6f498a', 0, @@ -336,7 +351,8 @@ describe('AssetsContractController', () => { const { assetsContract, messenger } = await setupAssetContractControllers(); assetsContract.provider = undefined; await expect( - assetsContract.getTokenStandardAndDetails( + messenger.call( + `AssetsContractController:getTokenStandardAndDetails`, ERC20_UNI_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, ), @@ -350,7 +366,8 @@ describe('AssetsContractController', () => { assetsContract.provider = provider; const error = 'Unable to determine contract standard'; await expect( - assetsContract.getTokenStandardAndDetails( + messenger.call( + `AssetsContractController:getTokenStandardAndDetails`, 'BaDeRc20AdDrEsS', TEST_ACCOUNT_PUBLIC_ADDRESS, ), @@ -415,7 +432,8 @@ describe('AssetsContractController', () => { }, ], }); - const standardAndDetails = await assetsContract.getTokenStandardAndDetails( + const standardAndDetails = await messenger.call( + `AssetsContractController:getTokenStandardAndDetails`, ERC721_GODS_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, ); @@ -496,7 +514,8 @@ describe('AssetsContractController', () => { }, ], }); - const standardAndDetails = await assetsContract.getTokenStandardAndDetails( + const standardAndDetails = await messenger.call( + `AssetsContractController:getTokenStandardAndDetails`, ERC1155_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, ); @@ -597,7 +616,8 @@ describe('AssetsContractController', () => { }, ], }); - const standardAndDetails = await assetsContract.getTokenStandardAndDetails( + const standardAndDetails = await messenger.call( + `AssetsContractController:getTokenStandardAndDetails`, ERC20_UNI_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, ); @@ -646,7 +666,8 @@ describe('AssetsContractController', () => { }, ], }); - const tokenId = await assetsContract.getERC721TokenURI( + const tokenId = await messenger.call( + `AssetsContractController:getERC721TokenURI`, ERC721_GODS_ADDRESS, '0', ); @@ -699,7 +720,8 @@ describe('AssetsContractController', () => { }, ], }); - const uri = await assetsContract.getERC721TokenURI( + const uri = await messenger.call( + `AssetsContractController:getERC721TokenURI`, '0x0000000000000000000000000000000000000000', '0', ); @@ -737,7 +759,10 @@ describe('AssetsContractController', () => { }, ], }); - const name = await assetsContract.getERC721AssetName(ERC721_GODS_ADDRESS); + const name = await messenger.call( + `AssetsContractController:getERC721AssetName`, + ERC721_GODS_ADDRESS, + ); expect(name).toBe('Gods Unchained'); messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); @@ -767,7 +792,8 @@ describe('AssetsContractController', () => { }, ], }); - const symbol = await assetsContract.getERC721AssetSymbol( + const symbol = await messenger.call( + `AssetsContractController:getERC721AssetSymbol`, ERC721_GODS_ADDRESS, ); expect(symbol).toBe('GODS'); @@ -775,9 +801,12 @@ describe('AssetsContractController', () => { }); it('should throw missing provider error when getting ERC-721 NFT symbol when missing provider', async () => { - const { assetsContract, messenger } = await setupAssetContractControllers(); + const { messenger } = await setupAssetContractControllers(); await expect( - assetsContract.getERC721AssetSymbol(ERC721_GODS_ADDRESS), + messenger.call( + `AssetsContractController:getERC721AssetSymbol`, + ERC721_GODS_ADDRESS, + ), ).rejects.toThrow(MISSING_PROVIDER_ERROR); messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); @@ -807,7 +836,8 @@ describe('AssetsContractController', () => { }, ], }); - const decimals = await assetsContract.getERC20TokenDecimals( + const decimals = await messenger.call( + `AssetsContractController:getERC20TokenDecimals`, ERC20_SAI_ADDRESS, ); expect(Number(decimals)).toBe(18); @@ -840,7 +870,10 @@ describe('AssetsContractController', () => { ], }); - const name = await assetsContract.getERC20TokenName(ERC20_DAI_ADDRESS); + const name = await messenger.call( + `AssetsContractController:getERC20TokenName`, + ERC20_DAI_ADDRESS, + ); expect(name).toBe('Dai Stablecoin'); messenger.clearEventSubscriptions('NetworkController:networkDidChange'); @@ -871,7 +904,8 @@ describe('AssetsContractController', () => { }, ], }); - const tokenId = await assetsContract.getERC721OwnerOf( + const tokenId = await messenger.call( + `AssetsContractController:getERC721OwnerOf`, ERC721_GODS_ADDRESS, '148332', ); @@ -880,9 +914,13 @@ describe('AssetsContractController', () => { }); it('should throw missing provider error when getting ERC-721 NFT ownership', async () => { - const { assetsContract, messenger } = await setupAssetContractControllers(); + const { messenger } = await setupAssetContractControllers(); await expect( - assetsContract.getERC721OwnerOf(ERC721_GODS_ADDRESS, '148332'), + messenger.call( + `AssetsContractController:getERC721OwnerOf`, + ERC721_GODS_ADDRESS, + '148332', + ), ).rejects.toThrow(MISSING_PROVIDER_ERROR); messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); @@ -912,7 +950,8 @@ describe('AssetsContractController', () => { }, ], }); - const balances = await assetsContract.getBalancesInSingleCall( + const balances = await messenger.call( + `AssetsContractController:getBalancesInSingleCall`, ERC20_SAI_ADDRESS, [ERC20_SAI_ADDRESS], ); @@ -988,7 +1027,7 @@ describe('AssetsContractController', () => { }, ], }); - const { assetsContract, network, provider } = + const { assetsContract, messenger, network, provider } = await setupAssetContractControllers({ options: { chainId: ChainId.mainnet, @@ -998,7 +1037,8 @@ describe('AssetsContractController', () => { }); assetsContract.provider = provider; - const balancesOnMainnet = await assetsContract.getBalancesInSingleCall( + const balancesOnMainnet = await messenger.call( + `AssetsContractController:getBalancesInSingleCall`, ERC20_SAI_ADDRESS, [ERC20_SAI_ADDRESS], ); @@ -1008,9 +1048,11 @@ describe('AssetsContractController', () => { await network.setActiveNetwork(InfuraNetworkType['linea-mainnet']); - const balancesOnLineaMainnet = await assetsContract.getBalancesInSingleCall( + const balancesOnLineaMainnet = await messenger.call( + `AssetsContractController:getBalancesInSingleCall`, ERC20_SAI_ADDRESS, [ERC20_SAI_ADDRESS], + InfuraNetworkType['linea-mainnet'], ); expect(balancesOnLineaMainnet).toStrictEqual({ [ERC20_SAI_ADDRESS]: BigNumber.from('0xa0155d09733ed8ef4c4'), @@ -1078,7 +1120,8 @@ describe('AssetsContractController', () => { ], }); - const balances = await assetsContract.getBalancesInSingleCall( + const balances = await messenger.call( + `AssetsContractController:getBalancesInSingleCall`, ERC20_SAI_ADDRESS, [ERC20_SAI_ADDRESS], ); @@ -1086,7 +1129,8 @@ describe('AssetsContractController', () => { await network.setActiveNetwork(NetworkType.sepolia); - const noBalances = await assetsContract.getBalancesInSingleCall( + const noBalances = await messenger.call( + `AssetsContractController:getBalancesInSingleCall`, ERC20_SAI_ADDRESS, [ERC20_SAI_ADDRESS], ); @@ -1098,7 +1142,8 @@ describe('AssetsContractController', () => { const { assetsContract, messenger } = await setupAssetContractControllers(); assetsContract.provider = undefined; await expect( - assetsContract.transferSingleERC1155( + messenger.call( + `AssetsContractController:transferSingleERC1155`, ERC1155_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, @@ -1135,7 +1180,8 @@ describe('AssetsContractController', () => { ], }); await expect( - assetsContract.transferSingleERC1155( + messenger.call( + `AssetsContractController:transferSingleERC1155`, ERC1155_ADDRESS, '0x0', TEST_ACCOUNT_PUBLIC_ADDRESS, @@ -1171,19 +1217,21 @@ describe('AssetsContractController', () => { }, ], }); - const balance = await assetsContract.getERC1155BalanceOf( + const balance = await messenger.call( + `AssetsContractController:getERC1155BalanceOf`, TEST_ACCOUNT_PUBLIC_ADDRESS, ERC1155_ADDRESS, ERC1155_ID, ); - expect(Number(balance)).toBeGreaterThan(0); + expect(balance.isZero() || balance.isNeg()).toBe(false); messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should throw missing provider error when getting the balance of a ERC-1155 NFT when missing provider', async () => { - const { assetsContract, messenger } = await setupAssetContractControllers(); + const { messenger } = await setupAssetContractControllers(); await expect( - assetsContract.getERC1155BalanceOf( + messenger.call( + `AssetsContractController:getERC1155BalanceOf`, TEST_ACCOUNT_PUBLIC_ADDRESS, ERC1155_ADDRESS, ERC1155_ID, @@ -1218,7 +1266,8 @@ describe('AssetsContractController', () => { ], }); const expectedUri = `https://api.opensea.io/api/v1/metadata/${ERC1155_ADDRESS}/0x{id}`; - const uri = await assetsContract.getERC1155TokenURI( + const uri = await messenger.call( + `AssetsContractController:getERC1155TokenURI`, ERC1155_ADDRESS, ERC1155_ID, ); diff --git a/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts b/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts index d064192b68..6daeafeae6 100644 --- a/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts +++ b/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts @@ -19,9 +19,10 @@ const TEST_ACCOUNT_PUBLIC_ADDRESS = describe('AssetsContractController with NetworkClientId', () => { it('should throw when getting ERC-20 token balance when networkClientId is invalid', async () => { - const { assetsContract, messenger } = await setupAssetContractControllers(); + const { messenger } = await setupAssetContractControllers(); await expect( - assetsContract.getERC20BalanceOf( + messenger.call( + `AssetsContractController:getERC20BalanceOf`, ERC20_UNI_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, 'invalidNetworkClientId', @@ -31,9 +32,10 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should throw when getting ERC-20 token decimal when networkClientId is invalid', async () => { - const { assetsContract, messenger } = await setupAssetContractControllers(); + const { messenger } = await setupAssetContractControllers(); await expect( - assetsContract.getERC20TokenDecimals( + messenger.call( + `AssetsContractController:getERC20TokenDecimals`, ERC20_UNI_ADDRESS, 'invalidNetworkClientId', ), @@ -42,7 +44,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get balance of ERC-20 token contract correctly', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -81,12 +83,14 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); - const UNIBalance = await assetsContract.getERC20BalanceOf( + const UNIBalance = await messenger.call( + `AssetsContractController:getERC20BalanceOf`, ERC20_UNI_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, 'mainnet', ); - const UNINoBalance = await assetsContract.getERC20BalanceOf( + const UNINoBalance = await messenger.call( + `AssetsContractController:getERC20BalanceOf`, ERC20_UNI_ADDRESS, '0x202637dAAEfbd7f131f90338a4A6c69F6Cd5CE91', 'mainnet', @@ -97,7 +101,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get ERC-721 NFT tokenId correctly', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -120,7 +124,8 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); - const tokenId = await assetsContract.getERC721NftTokenId( + const tokenId = await messenger.call( + `AssetsContractController:getERC721NftTokenId`, ERC721_GODS_ADDRESS, '0x9a90bd8d1149a88b42a99cf62215ad955d6f498a', 0, @@ -131,9 +136,10 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should throw error when getting ERC-721 token standard and details when networkClientId is invalid', async () => { - const { assetsContract, messenger } = await setupAssetContractControllers(); + const { messenger } = await setupAssetContractControllers(); await expect( - assetsContract.getTokenStandardAndDetails( + messenger.call( + `AssetsContractController:getTokenStandardAndDetails`, ERC20_UNI_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, undefined, @@ -144,10 +150,11 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should throw contract standard error when getting ERC-20 token standard and details when provided with invalid ERC-20 address', async () => { - const { assetsContract, messenger } = await setupAssetContractControllers(); + const { messenger } = await setupAssetContractControllers(); const error = 'Unable to determine contract standard'; await expect( - assetsContract.getTokenStandardAndDetails( + messenger.call( + `AssetsContractController:getTokenStandardAndDetails`, 'BaDeRc20AdDrEsS', TEST_ACCOUNT_PUBLIC_ADDRESS, undefined, @@ -158,7 +165,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get ERC-721 token standard and details', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -213,7 +220,8 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); - const standardAndDetails = await assetsContract.getTokenStandardAndDetails( + const standardAndDetails = await messenger.call( + `AssetsContractController:getTokenStandardAndDetails`, ERC721_GODS_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, undefined, @@ -224,7 +232,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get ERC-1155 token standard and details', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -263,7 +271,8 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); - const standardAndDetails = await assetsContract.getTokenStandardAndDetails( + const standardAndDetails = await messenger.call( + `AssetsContractController:getTokenStandardAndDetails`, ERC1155_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, undefined, @@ -274,7 +283,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get ERC-20 token standard and details', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -361,7 +370,8 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); - const standardAndDetails = await assetsContract.getTokenStandardAndDetails( + const standardAndDetails = await messenger.call( + `AssetsContractController:getTokenStandardAndDetails`, ERC20_UNI_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, undefined, @@ -372,7 +382,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get ERC-721 NFT tokenURI correctly', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -411,7 +421,8 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); - const tokenId = await assetsContract.getERC721TokenURI( + const tokenId = await messenger.call( + `AssetsContractController:getERC721TokenURI`, ERC721_GODS_ADDRESS, '0', 'mainnet', @@ -421,7 +432,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should not throw an error when address given is does not support NFT Metadata interface', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -464,7 +475,8 @@ describe('AssetsContractController with NetworkClientId', () => { .mockImplementationOnce(() => { /**/ }); - const uri = await assetsContract.getERC721TokenURI( + const uri = await messenger.call( + `AssetsContractController:getERC721TokenURI`, '0x0000000000000000000000000000000000000000', '0', 'mainnet', @@ -478,7 +490,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get ERC-721 NFT name', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -501,7 +513,8 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); - const name = await assetsContract.getERC721AssetName( + const name = await messenger.call( + `AssetsContractController:getERC721AssetName`, ERC721_GODS_ADDRESS, 'mainnet', ); @@ -510,7 +523,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get ERC-721 NFT symbol', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -533,7 +546,8 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); - const symbol = await assetsContract.getERC721AssetSymbol( + const symbol = await messenger.call( + `AssetsContractController:getERC721AssetSymbol`, ERC721_GODS_ADDRESS, 'mainnet', ); @@ -542,9 +556,10 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should throw error when getting ERC-721 NFT symbol when networkClientId is invalid', async () => { - const { assetsContract, messenger } = await setupAssetContractControllers(); + const { messenger } = await setupAssetContractControllers(); await expect( - assetsContract.getERC721AssetSymbol( + messenger.call( + `AssetsContractController:getERC721AssetSymbol`, ERC721_GODS_ADDRESS, 'invalidNetworkClientId', ), @@ -553,7 +568,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get ERC-20 token decimals', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -576,7 +591,8 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); - const decimals = await assetsContract.getERC20TokenDecimals( + const decimals = await messenger.call( + `AssetsContractController:getERC20TokenDecimals`, ERC20_SAI_ADDRESS, 'mainnet', ); @@ -585,7 +601,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get ERC-20 token name', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -609,7 +625,8 @@ describe('AssetsContractController with NetworkClientId', () => { ], }); - const name = await assetsContract.getERC20TokenName( + const name = await messenger.call( + `AssetsContractController:getERC20TokenName`, ERC20_DAI_ADDRESS, 'mainnet', ); @@ -619,7 +636,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get ERC-721 NFT ownership', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -642,7 +659,8 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); - const tokenId = await assetsContract.getERC721OwnerOf( + const tokenId = await messenger.call( + `AssetsContractController:getERC721OwnerOf`, ERC721_GODS_ADDRESS, '148332', 'mainnet', @@ -652,9 +670,10 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should throw error when getting ERC-721 NFT ownership using networkClientId that is invalid', async () => { - const { assetsContract, messenger } = await setupAssetContractControllers(); + const { messenger } = await setupAssetContractControllers(); await expect( - assetsContract.getERC721OwnerOf( + messenger.call( + `AssetsContractController:getERC721OwnerOf`, ERC721_GODS_ADDRESS, '148332', 'invalidNetworkClientId', @@ -664,7 +683,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get balance of ERC-20 token in a single call on network with token detection support', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -687,7 +706,8 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); - const balances = await assetsContract.getBalancesInSingleCall( + const balances = await messenger.call( + `AssetsContractController:getBalancesInSingleCall`, ERC20_SAI_ADDRESS, [ERC20_SAI_ADDRESS], 'mainnet', @@ -697,7 +717,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should not have balance in a single call after switching to network without token detection support', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -751,14 +771,16 @@ describe('AssetsContractController with NetworkClientId', () => { ], }); - const balances = await assetsContract.getBalancesInSingleCall( + const balances = await messenger.call( + `AssetsContractController:getBalancesInSingleCall`, ERC20_SAI_ADDRESS, [ERC20_SAI_ADDRESS], 'mainnet', ); expect(balances[ERC20_SAI_ADDRESS]).toBeDefined(); - const noBalances = await assetsContract.getBalancesInSingleCall( + const noBalances = await messenger.call( + `AssetsContractController:getBalancesInSingleCall`, ERC20_SAI_ADDRESS, [ERC20_SAI_ADDRESS], 'sepolia', @@ -768,9 +790,10 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should throw error when transferring single ERC-1155 when networkClientId is invalid', async () => { - const { assetsContract, messenger } = await setupAssetContractControllers(); + const { messenger } = await setupAssetContractControllers(); await expect( - assetsContract.transferSingleERC1155( + messenger.call( + `AssetsContractController:transferSingleERC1155`, ERC1155_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, TEST_ACCOUNT_PUBLIC_ADDRESS, @@ -783,7 +806,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get the balance of a ERC-1155 NFT for a given address', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -806,7 +829,8 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); - const balance = await assetsContract.getERC1155BalanceOf( + const balance = await messenger.call( + `AssetsContractController:getERC1155BalanceOf`, TEST_ACCOUNT_PUBLIC_ADDRESS, ERC1155_ADDRESS, ERC1155_ID, @@ -817,9 +841,10 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should throw error when getting the balance of a ERC-1155 NFT when networkClientId is invalid', async () => { - const { assetsContract, messenger } = await setupAssetContractControllers(); + const { messenger } = await setupAssetContractControllers(); await expect( - assetsContract.getERC1155BalanceOf( + messenger.call( + `AssetsContractController:getERC1155BalanceOf`, TEST_ACCOUNT_PUBLIC_ADDRESS, ERC1155_ADDRESS, ERC1155_ID, @@ -830,7 +855,7 @@ describe('AssetsContractController with NetworkClientId', () => { }); it('should get the URI of a ERC-1155 NFT', async () => { - const { assetsContract, messenger, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ networkClientConfiguration, @@ -854,7 +879,8 @@ describe('AssetsContractController with NetworkClientId', () => { ], }); const expectedUri = `https://api.opensea.io/api/v1/metadata/${ERC1155_ADDRESS}/0x{id}`; - const uri = await assetsContract.getERC1155TokenURI( + const uri = await messenger.call( + `AssetsContractController:getERC1155TokenURI`, ERC1155_ADDRESS, ERC1155_ID, 'mainnet', diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 69e47377fd..327c057c90 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -1,15 +1,6 @@ import type { Network } from '@ethersproject/providers'; -import type { - AccountsControllerGetAccountAction, - AccountsControllerGetSelectedAccountAction, - AccountsControllerSelectedAccountChangeEvent, - AccountsControllerSelectedEvmAccountChangeEvent, -} from '@metamask/accounts-controller'; -import type { - AddApprovalRequest, - ApprovalStateChange, - ApprovalControllerMessenger, -} from '@metamask/approval-controller'; +import type { AccountsControllerSelectedAccountChangeEvent } from '@metamask/accounts-controller'; +import type { ApprovalControllerMessenger } from '@metamask/approval-controller'; import { ApprovalController } from '@metamask/approval-controller'; import { ControllerMessenger } from '@metamask/base-controller'; import { @@ -29,11 +20,8 @@ import type { InternalAccount } from '@metamask/keyring-api'; import type { NetworkClientConfiguration, NetworkClientId, - NetworkControllerGetNetworkClientByIdAction, - NetworkControllerNetworkDidChangeEvent, } from '@metamask/network-controller'; import { defaultState as defaultNetworkState } from '@metamask/network-controller'; -import type { PreferencesControllerStateChangeEvent } from '@metamask/preferences-controller'; import { getDefaultPreferencesState, type PreferencesState, @@ -52,6 +40,14 @@ import { buildCustomNetworkClientConfiguration, buildMockGetNetworkClientById, } from '../../network-controller/tests/helpers'; +import type { + AssetsContractControllerGetERC1155BalanceOfAction, + AssetsContractControllerGetERC1155TokenURIAction, + AssetsContractControllerGetERC721AssetNameAction, + AssetsContractControllerGetERC721AssetSymbolAction, + AssetsContractControllerGetERC721OwnerOfAction, + AssetsContractControllerGetERC721TokenURIAction, +} from './AssetsContractController'; import { getFormattedIpfsUrl } from './assetsUtil'; import { Source } from './constants'; import type { @@ -104,17 +100,6 @@ const GOERLI = { ticker: NetworksTicker.goerli, }; -type ApprovalActions = - | AddApprovalRequest - | AccountsControllerGetAccountAction - | AccountsControllerGetSelectedAccountAction - | NetworkControllerGetNetworkClientByIdAction; -type ApprovalEvents = - | ApprovalStateChange - | PreferencesControllerStateChangeEvent - | NetworkControllerNetworkDidChangeEvent - | AccountsControllerSelectedEvmAccountChangeEvent; - const controllerName = 'NftController' as const; // Mock out detectNetwork function for cleaner tests, Ethers calls this a bunch of times because the Web3Provider is paranoid. @@ -148,6 +133,18 @@ jest.mock('uuid', () => { * * @param args - Arguments to this function. * @param args.options - Controller options. + * @param args.getERC721AssetName - Used to construct mock versions of the + * `AssetsContractController:getERC721AssetName` action. + * @param args.getERC721AssetSymbol - Used to construct mock versions of the + * `AssetsContractController:getERC721AssetSymbol` action. + * @param args.getERC721TokenURI - Used to construct mock versions of the + * `AssetsContractController:getERC721TokenURI` action. + * @param args.getERC721OwnerOf - Used to construct mock versions of the + * `AssetsContractController:getERC721OwnerOf` action. + * @param args.getERC1155BalanceOf - Used to construct mock versions of the + * `AssetsContractController:getERC1155BalanceOf` action. + * @param args.getERC1155TokenURI - Used to construct mock versions of the + * `AssetsContractController:getERC1155TokenURI` action. * @param args.mockNetworkClientConfigurationsByNetworkClientId - Used to construct * mock versions of network clients and ultimately mock the * `NetworkController:getNetworkClientById` action. @@ -156,10 +153,40 @@ jest.mock('uuid', () => { */ function setupController({ options = {}, + getERC721AssetName, + getERC721AssetSymbol, + getERC721TokenURI, + getERC721OwnerOf, + getERC1155BalanceOf, + getERC1155TokenURI, mockNetworkClientConfigurationsByNetworkClientId = {}, defaultSelectedAccount = OWNER_ACCOUNT, }: { options?: Partial[0]>; + getERC721AssetName?: jest.Mock< + ReturnType, + Parameters + >; + getERC721AssetSymbol?: jest.Mock< + ReturnType, + Parameters + >; + getERC721TokenURI?: jest.Mock< + ReturnType, + Parameters + >; + getERC721OwnerOf?: jest.Mock< + ReturnType, + Parameters + >; + getERC1155BalanceOf?: jest.Mock< + ReturnType, + Parameters + >; + getERC1155TokenURI?: jest.Mock< + ReturnType, + Parameters + >; mockNetworkClientConfigurationsByNetworkClientId?: Record< NetworkClientId, NetworkClientConfiguration @@ -202,6 +229,72 @@ function setupController({ mockGetSelectedAccount, ); + const mockGetERC721AssetName = + getERC721AssetName ?? + jest.fn< + ReturnType, + Parameters + >(); + messenger.registerActionHandler( + 'AssetsContractController:getERC721AssetName', + mockGetERC721AssetName, + ); + + const mockGetERC721AssetSymbol = + getERC721AssetSymbol ?? + jest.fn< + ReturnType, + Parameters + >(); + messenger.registerActionHandler( + 'AssetsContractController:getERC721AssetSymbol', + mockGetERC721AssetSymbol, + ); + + const mockGetERC721TokenURI = + getERC721TokenURI ?? + jest.fn< + ReturnType, + Parameters + >(); + messenger.registerActionHandler( + 'AssetsContractController:getERC721TokenURI', + mockGetERC721TokenURI, + ); + + const mockGetERC721OwnerOf = + getERC721OwnerOf ?? + jest.fn< + ReturnType, + Parameters + >(); + messenger.registerActionHandler( + 'AssetsContractController:getERC721OwnerOf', + mockGetERC721OwnerOf, + ); + + const mockGetERC1155BalanceOf = + getERC1155BalanceOf ?? + jest.fn< + ReturnType, + Parameters + >(); + messenger.registerActionHandler( + 'AssetsContractController:getERC1155BalanceOf', + mockGetERC1155BalanceOf, + ); + + const mockGetERC1155TokenURI = + getERC1155TokenURI ?? + jest.fn< + ReturnType, + Parameters + >(); + messenger.registerActionHandler( + 'AssetsContractController:getERC1155TokenURI', + mockGetERC1155TokenURI, + ); + const approvalControllerMessenger = messenger.getRestricted({ name: 'ApprovalController', allowedActions: [], @@ -213,25 +306,21 @@ function setupController({ showApprovalRequest: jest.fn(), }); - const nftControllerMessenger = messenger.getRestricted< - typeof controllerName, - ApprovalActions['type'], - Extract< - ApprovalEvents, - | PreferencesControllerStateChangeEvent - | AccountsControllerSelectedEvmAccountChangeEvent - | NetworkControllerNetworkDidChangeEvent - >['type'] - >({ + const nftControllerMessenger = messenger.getRestricted({ name: controllerName, allowedActions: [ 'ApprovalController:addRequest', 'AccountsController:getSelectedAccount', 'AccountsController:getAccount', 'NetworkController:getNetworkClientById', + 'AssetsContractController:getERC721AssetName', + 'AssetsContractController:getERC721AssetSymbol', + 'AssetsContractController:getERC721TokenURI', + 'AssetsContractController:getERC721OwnerOf', + 'AssetsContractController:getERC1155BalanceOf', + 'AssetsContractController:getERC1155TokenURI', ], allowedEvents: [ - // @ts-expect-error - Adding this for test 'AccountsController:selectedAccountChange', 'AccountsController:selectedEvmAccountChange', 'PreferencesController:stateChange', @@ -241,13 +330,8 @@ function setupController({ const nftController = new NftController({ chainId: ChainId.mainnet, - getERC721AssetName: jest.fn(), - getERC721AssetSymbol: jest.fn(), - getERC721TokenURI: jest.fn(), - getERC721OwnerOf: jest.fn(), - getERC1155BalanceOf: jest.fn(), - getERC1155TokenURI: jest.fn(), onNftAdded: jest.fn(), + // @ts-expect-error - Added incompatible event `AccountsController:selectedAccountChange` to allowlist for testing purposes messenger: nftControllerMessenger, ...options, }); @@ -292,6 +376,12 @@ function setupController({ triggerSelectedAccountChange, mockGetAccount, mockGetSelectedAccount, + mockGetERC1155BalanceOf, + mockGetERC1155TokenURI, + mockGetERC721AssetName, + mockGetERC721AssetSymbol, + mockGetERC721OwnerOf, + mockGetERC721TokenURI, }; } @@ -408,12 +498,10 @@ describe('NftController', () => { }), ); const { nftController } = setupController({ - options: { - getERC721TokenURI: jest - .fn() - .mockImplementation(() => 'https://testtokenuri.com'), - getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), - }, + getERC721TokenURI: jest + .fn() + .mockImplementation(() => 'https://testtokenuri.com'), + getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), }); // eslint-disable-next-line @typescript-eslint/ban-ts-comment @@ -485,9 +573,7 @@ describe('NftController', () => { it('should error if the user does not own the suggested ERC721 NFT', async function () { const { nftController, messenger } = setupController({ - options: { - getERC721OwnerOf: jest.fn().mockImplementation(() => '0x12345abcefg'), - }, + getERC721OwnerOf: jest.fn().mockImplementation(() => '0x12345abcefg'), }); const callActionSpy = jest.spyOn(messenger, 'call'); @@ -520,9 +606,7 @@ describe('NftController', () => { it('should error if the user does not own the suggested ERC1155 NFT', async function () { const { nftController, messenger } = setupController({ - options: { - getERC1155BalanceOf: jest.fn().mockImplementation(() => new BN(0)), - }, + getERC1155BalanceOf: jest.fn().mockImplementation(() => new BN(0)), }); const callActionSpy = jest.spyOn(messenger, 'call'); @@ -531,7 +615,11 @@ describe('NftController', () => { nftController.watchNft(ERC1155_NFT, ERC1155, 'https://test-dapp.com'), ).rejects.toThrow('Suggested NFT is not owned by the selected account'); // First call is to get InternalAccount - expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenNthCalledWith( + 1, + 'AccountsController:getAccount', + expect.any(String), + ); }); it('should handle ERC721 type and add pending request to ApprovalController with the OpenSea API disabled and IPFS gateway enabled', async function () { @@ -551,12 +639,10 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ - options: { - getERC721TokenURI: jest - .fn() - .mockImplementation(() => 'https://testtokenuri.com'), - getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), - }, + getERC721TokenURI: jest + .fn() + .mockImplementation(() => 'https://testtokenuri.com'), + getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), }); triggerSelectedAccountChange(OWNER_ACCOUNT); triggerPreferencesStateChange({ @@ -622,12 +708,10 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ - options: { - getERC721TokenURI: jest - .fn() - .mockImplementation(() => 'https://testtokenuri.com'), - getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), - }, + getERC721TokenURI: jest + .fn() + .mockImplementation(() => 'https://testtokenuri.com'), + getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), }); triggerSelectedAccountChange(OWNER_ACCOUNT); triggerPreferencesStateChange({ @@ -693,12 +777,10 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ - options: { - getERC721TokenURI: jest - .fn() - .mockImplementation(() => 'ipfs://testtokenuri.com'), - getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), - }, + getERC721TokenURI: jest + .fn() + .mockImplementation(() => 'ipfs://testtokenuri.com'), + getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), }); triggerSelectedAccountChange(OWNER_ACCOUNT); triggerPreferencesStateChange({ @@ -764,12 +846,10 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ - options: { - getERC721TokenURI: jest - .fn() - .mockImplementation(() => 'ipfs://testtokenuri.com'), - getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), - }, + getERC721TokenURI: jest + .fn() + .mockImplementation(() => 'ipfs://testtokenuri.com'), + getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), }); triggerSelectedAccountChange(OWNER_ACCOUNT); @@ -837,15 +917,13 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ - options: { - getERC721TokenURI: jest - .fn() - .mockRejectedValue(new Error('Not an ERC721 contract')), - getERC1155TokenURI: jest - .fn() - .mockImplementation(() => 'https://testtokenuri.com'), - getERC1155BalanceOf: jest.fn().mockImplementation(() => new BN(1)), - }, + getERC721TokenURI: jest + .fn() + .mockRejectedValue(new Error('Not an ERC721 contract')), + getERC1155TokenURI: jest + .fn() + .mockImplementation(() => 'https://testtokenuri.com'), + getERC1155BalanceOf: jest.fn().mockImplementation(() => new BN(1)), }); triggerSelectedAccountChange(OWNER_ACCOUNT); @@ -912,15 +990,13 @@ describe('NftController', () => { const { nftController, messenger, triggerPreferencesStateChange } = setupController({ - options: { - getERC721TokenURI: jest - .fn() - .mockRejectedValue(new Error('Not an ERC721 contract')), - getERC1155TokenURI: jest - .fn() - .mockImplementation(() => 'https://testtokenuri.com'), - getERC1155BalanceOf: jest.fn().mockImplementation(() => new BN(1)), - }, + getERC721TokenURI: jest + .fn() + .mockRejectedValue(new Error('Not an ERC721 contract')), + getERC1155TokenURI: jest + .fn() + .mockImplementation(() => 'https://testtokenuri.com'), + getERC1155BalanceOf: jest.fn().mockImplementation(() => new BN(1)), }); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), @@ -991,20 +1067,18 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ - options: { - getERC721OwnerOf: jest - .fn() - .mockImplementation(() => SECOND_OWNER_ADDRESS), - getERC721TokenURI: jest - .fn() - .mockImplementation(() => 'https://testtokenuri.com'), - getERC721AssetName: jest - .fn() - .mockImplementation(() => 'testERC721Name'), - getERC721AssetSymbol: jest - .fn() - .mockImplementation(() => 'testERC721Symbol'), - }, + getERC721OwnerOf: jest + .fn() + .mockImplementation(() => SECOND_OWNER_ADDRESS), + getERC721TokenURI: jest + .fn() + .mockImplementation(() => 'https://testtokenuri.com'), + getERC721AssetName: jest + .fn() + .mockImplementation(() => 'testERC721Name'), + getERC721AssetSymbol: jest + .fn() + .mockImplementation(() => 'testERC721Symbol'), }); const requestId = 'approval-request-id-1'; @@ -1097,18 +1171,16 @@ describe('NftController', () => { triggerSelectedAccountChange, changeNetwork, } = setupController({ - options: { - getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), - getERC721TokenURI: jest - .fn() - .mockImplementation(() => 'https://testtokenuri.com'), - getERC721AssetName: jest - .fn() - .mockImplementation(() => 'testERC721Name'), - getERC721AssetSymbol: jest - .fn() - .mockImplementation(() => 'testERC721Symbol'), - }, + getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), + getERC721TokenURI: jest + .fn() + .mockImplementation(() => 'https://testtokenuri.com'), + getERC721AssetName: jest + .fn() + .mockImplementation(() => 'testERC721Name'), + getERC721AssetSymbol: jest + .fn() + .mockImplementation(() => 'testERC721Symbol'), }); const requestId = 'approval-request-id-1'; @@ -1220,8 +1292,8 @@ describe('NftController', () => { const { nftController } = setupController({ options: { chainId: ChainId.mainnet, - getERC721AssetName: jest.fn().mockResolvedValue('Name'), }, + getERC721AssetName: jest.fn().mockResolvedValue('Name'), }); await nftController.addNft('0x01', '1', { @@ -1334,10 +1406,8 @@ describe('NftController', () => { triggerSelectedAccountChange, mockGetAccount, } = setupController({ - options: { - getERC721TokenURI: mockGetERC721TokenURI, - getERC1155TokenURI: mockGetERC1155TokenURI, - }, + getERC721TokenURI: mockGetERC721TokenURI, + getERC1155TokenURI: mockGetERC1155TokenURI, }); const firstAddress = '0x123'; const firstAccount = createMockInternalAccount({ address: firstAddress }); @@ -1388,7 +1458,6 @@ describe('NftController', () => { it('should update NFT if image is different', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -1584,7 +1653,6 @@ describe('NftController', () => { it('should not duplicate NFT nor NFT contract if already added', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -1621,14 +1689,12 @@ describe('NftController', () => { it('should add NFT and get information from NFT-API', async () => { const { nftController } = setupController({ - options: { - getERC721TokenURI: jest - .fn() - .mockRejectedValue(new Error('Not an ERC721 contract')), - getERC1155TokenURI: jest - .fn() - .mockRejectedValue(new Error('Not an ERC1155 contract')), - }, + getERC721TokenURI: jest + .fn() + .mockRejectedValue(new Error('Not an ERC721 contract')), + getERC1155TokenURI: jest + .fn() + .mockRejectedValue(new Error('Not an ERC1155 contract')), defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -1701,15 +1767,13 @@ describe('NftController', () => { it('should add NFT erc721 and aggregate NFT data from both contract and NFT-API even if call to Get Collections fails', async () => { const { nftController } = setupController({ - options: { - getERC721AssetName: jest.fn().mockResolvedValue('KudosToken'), - getERC721AssetSymbol: jest.fn().mockResolvedValue('KDO'), - getERC721TokenURI: jest - .fn() - .mockResolvedValue( - 'https://ipfs.gitcoin.co:443/api/v0/cat/QmPmt6EAaioN78ECnW5oCL8v2YvVSpoBjLCjrXhhsAvoov', - ), - }, + getERC721AssetName: jest.fn().mockResolvedValue('KudosToken'), + getERC721AssetSymbol: jest.fn().mockResolvedValue('KDO'), + getERC721TokenURI: jest + .fn() + .mockResolvedValue( + 'https://ipfs.gitcoin.co:443/api/v0/cat/QmPmt6EAaioN78ECnW5oCL8v2YvVSpoBjLCjrXhhsAvoov', + ), defaultSelectedAccount: OWNER_ACCOUNT, }); nock(NFT_API_BASE_URL) @@ -1875,16 +1939,14 @@ describe('NftController', () => { it('should add NFT erc1155 and get NFT information from contract when NFT API call fail', async () => { const { nftController } = setupController({ - options: { - getERC721TokenURI: jest - .fn() - .mockRejectedValue(new Error('Not a 721 contract')), - getERC1155TokenURI: jest - .fn() - .mockResolvedValue( - 'https://api.opensea.io/api/v1/metadata/0x495f947276749Ce646f68AC8c248420045cb7b5e/0x{id}', - ), - }, + getERC721TokenURI: jest + .fn() + .mockRejectedValue(new Error('Not a 721 contract')), + getERC1155TokenURI: jest + .fn() + .mockResolvedValue( + 'https://api.opensea.io/api/v1/metadata/0x495f947276749Ce646f68AC8c248420045cb7b5e/0x{id}', + ), defaultSelectedAccount: OWNER_ACCOUNT, }); nock('https://api.opensea.io') @@ -1923,18 +1985,16 @@ describe('NftController', () => { it('should add NFT erc721 and get NFT information only from contract', async () => { const { nftController } = setupController({ - options: { - getERC721AssetName: jest.fn().mockResolvedValue('KudosToken'), - getERC721AssetSymbol: jest.fn().mockResolvedValue('KDO'), - getERC721TokenURI: jest.fn().mockImplementation((tokenAddress) => { - switch (tokenAddress) { - case ERC721_KUDOSADDRESS: - return 'https://ipfs.gitcoin.co:443/api/v0/cat/QmPmt6EAaioN78ECnW5oCL8v2YvVSpoBjLCjrXhhsAvoov'; - default: - throw new Error('Not an ERC721 token'); - } - }), - }, + getERC721AssetName: jest.fn().mockResolvedValue('KudosToken'), + getERC721AssetSymbol: jest.fn().mockResolvedValue('KDO'), + getERC721TokenURI: jest.fn().mockImplementation((tokenAddress) => { + switch (tokenAddress) { + case ERC721_KUDOSADDRESS: + return 'https://ipfs.gitcoin.co:443/api/v0/cat/QmPmt6EAaioN78ECnW5oCL8v2YvVSpoBjLCjrXhhsAvoov'; + default: + throw new Error('Not an ERC721 token'); + } + }), defaultSelectedAccount: OWNER_ACCOUNT, }); nock('https://ipfs.gitcoin.co:443') @@ -1984,11 +2044,9 @@ describe('NftController', () => { const testTokenUriEncoded = 'data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHByZXNlcnZlQXNwZWN0UmF0aW89InhNaW5ZTWluIG1l'; const { nftController } = setupController({ - options: { - getERC721AssetName: jest.fn().mockResolvedValue('KudosToken'), - getERC721AssetSymbol: jest.fn().mockResolvedValue('KDO'), - getERC721TokenURI: jest.fn().mockResolvedValue(testTokenUriEncoded), - }, + getERC721AssetName: jest.fn().mockResolvedValue('KudosToken'), + getERC721AssetSymbol: jest.fn().mockResolvedValue('KDO'), + getERC721TokenURI: jest.fn().mockResolvedValue(testTokenUriEncoded), defaultSelectedAccount: OWNER_ACCOUNT, }); await nftController.addNft(ERC721_KUDOSADDRESS, ERC721_KUDOS_TOKEN_ID); @@ -2012,9 +2070,7 @@ describe('NftController', () => { const tokenURI = 'https://url/'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, changeNetwork } = setupController({ - options: { - getERC721TokenURI: mockGetERC721TokenURI, - }, + getERC721TokenURI: mockGetERC721TokenURI, defaultSelectedAccount: OWNER_ACCOUNT, }); nock('https://url').get('/').reply(200, { @@ -2060,10 +2116,10 @@ describe('NftController', () => { const { nftController } = setupController({ options: { onNftAdded: mockOnNftAdded, - getERC721AssetSymbol: mockGetERC721AssetSymbol, - getERC721AssetName: mockGetERC721AssetName, - getERC721TokenURI: mockGetERC721TokenURI, }, + getERC721AssetSymbol: mockGetERC721AssetSymbol, + getERC721AssetName: mockGetERC721AssetName, + getERC721TokenURI: mockGetERC721TokenURI, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -2122,10 +2178,10 @@ describe('NftController', () => { const { nftController, changeNetwork } = setupController({ options: { onNftAdded: mockOnNftAdded, - getERC721AssetSymbol: mockGetERC721AssetSymbol, - getERC721AssetName: mockGetERC721AssetName, - getERC721TokenURI: mockGetERC721TokenURI, }, + getERC721AssetSymbol: mockGetERC721AssetSymbol, + getERC721AssetName: mockGetERC721AssetName, + getERC721TokenURI: mockGetERC721TokenURI, }); nock('https://url').get('/').reply(200, { name: 'name', @@ -2182,13 +2238,13 @@ describe('NftController', () => { const { nftController } = setupController({ options: { onNftAdded: mockOnNftAdded, - getERC721AssetName: jest - .fn() - .mockRejectedValue(new Error('Failed to fetch')), - getERC721AssetSymbol: jest - .fn() - .mockRejectedValue(new Error('Failed to fetch')), }, + getERC721AssetName: jest + .fn() + .mockRejectedValue(new Error('Failed to fetch')), + getERC721AssetSymbol: jest + .fn() + .mockRejectedValue(new Error('Failed to fetch')), defaultSelectedAccount: OWNER_ACCOUNT, }); nock(NFT_API_BASE_URL) @@ -2419,13 +2475,13 @@ describe('NftController', () => { const { nftController } = setupController({ options: { onNftAdded: mockOnNftAdded, - getERC721AssetName: jest - .fn() - .mockRejectedValue(new Error('Failed to fetch')), - getERC721AssetSymbol: jest - .fn() - .mockRejectedValue(new Error('Failed to fetch')), }, + getERC721AssetName: jest + .fn() + .mockRejectedValue(new Error('Failed to fetch')), + getERC721AssetSymbol: jest + .fn() + .mockRejectedValue(new Error('Failed to fetch')), defaultSelectedAccount: OWNER_ACCOUNT, }); nock(NFT_API_BASE_URL) @@ -2453,7 +2509,6 @@ describe('NftController', () => { it('should not add duplicate NFTs to the ignoredNfts list', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -2510,23 +2565,21 @@ describe('NftController', () => { it('should add NFT with metadata hosted in IPFS', async () => { const { nftController, triggerPreferencesStateChange, mockGetAccount } = setupController({ - options: { - getERC721AssetName: jest - .fn() - .mockResolvedValue("Maltjik.jpg's Depressionists"), - getERC721AssetSymbol: jest.fn().mockResolvedValue('DPNS'), - getERC721TokenURI: jest.fn().mockImplementation((tokenAddress) => { - switch (tokenAddress) { - case ERC721_DEPRESSIONIST_ADDRESS: - return `ipfs://${DEPRESSIONIST_CID_V1}`; - default: - throw new Error('Not an ERC721 token'); - } - }), - getERC1155TokenURI: jest - .fn() - .mockRejectedValue(new Error('Not an ERC1155 token')), - }, + getERC721AssetName: jest + .fn() + .mockResolvedValue("Maltjik.jpg's Depressionists"), + getERC721AssetSymbol: jest.fn().mockResolvedValue('DPNS'), + getERC721TokenURI: jest.fn().mockImplementation((tokenAddress) => { + switch (tokenAddress) { + case ERC721_DEPRESSIONIST_ADDRESS: + return `ipfs://${DEPRESSIONIST_CID_V1}`; + default: + throw new Error('Not an ERC721 token'); + } + }), + getERC1155TokenURI: jest + .fn() + .mockRejectedValue(new Error('Not an ERC1155 token')), }); mockGetAccount.mockReturnValue(OWNER_ACCOUNT); triggerPreferencesStateChange({ @@ -2625,26 +2678,24 @@ describe('NftController', () => { ); const { nftController } = setupController({ - options: { - getERC721TokenURI: jest.fn().mockImplementation((tokenAddress) => { - switch (tokenAddress) { - case '0x01': - return 'https://testtokenuri-1.com'; - case '0x02': - return 'https://testtokenuri-2.com'; - default: - throw new Error('Not an ERC721 token'); - } - }), - getERC1155TokenURI: jest.fn().mockImplementation((tokenAddress) => { - switch (tokenAddress) { - case '0x03': - return 'https://testtokenuri-3.com'; - default: - throw new Error('Not an ERC1155 token'); - } - }), - }, + getERC721TokenURI: jest.fn().mockImplementation((tokenAddress) => { + switch (tokenAddress) { + case '0x01': + return 'https://testtokenuri-1.com'; + case '0x02': + return 'https://testtokenuri-2.com'; + default: + throw new Error('Not an ERC721 token'); + } + }), + getERC1155TokenURI: jest.fn().mockImplementation((tokenAddress) => { + switch (tokenAddress) { + case '0x03': + return 'https://testtokenuri-3.com'; + default: + throw new Error('Not an ERC1155 token'); + } + }), mockNetworkClientConfigurationsByNetworkClientId: { 'customNetworkClientId-1': buildCustomNetworkClientConfiguration({ chainId: '0xa', @@ -2745,26 +2796,24 @@ describe('NftController', () => { ); const { nftController, changeNetwork } = setupController({ - options: { - getERC721TokenURI: jest.fn().mockImplementation((tokenAddress) => { - switch (tokenAddress) { - case '0x01': - return 'https://testtokenuri-1.com'; - case '0x02': - return 'https://testtokenuri-2.com'; - default: - throw new Error('Not an ERC721 token'); - } - }), - getERC1155TokenURI: jest.fn().mockImplementation((tokenAddress) => { - switch (tokenAddress) { - case '0x03': - return 'https://testtokenuri-3.com'; - default: - throw new Error('Not an ERC1155 token'); - } - }), - }, + getERC721TokenURI: jest.fn().mockImplementation((tokenAddress) => { + switch (tokenAddress) { + case '0x01': + return 'https://testtokenuri-1.com'; + case '0x02': + return 'https://testtokenuri-2.com'; + default: + throw new Error('Not an ERC721 token'); + } + }), + getERC1155TokenURI: jest.fn().mockImplementation((tokenAddress) => { + switch (tokenAddress) { + case '0x03': + return 'https://testtokenuri-3.com'; + default: + throw new Error('Not an ERC1155 token'); + } + }), }); await nftController.addNft('0x01', '1234', { @@ -2831,8 +2880,8 @@ describe('NftController', () => { const { nftController, mockGetAccount } = setupController({ options: { chainId: ChainId.mainnet, - getERC721AssetName: jest.fn().mockResolvedValue('Name'), }, + getERC721AssetName: jest.fn().mockResolvedValue('Name'), }); mockGetAccount.mockReturnValue(null); @@ -2866,9 +2915,7 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ - options: { - getERC721TokenURI: mockGetERC721TokenURI, - }, + getERC721TokenURI: mockGetERC721TokenURI, }); const firstAddress = '0x123'; const firstAccount = createMockInternalAccount({ @@ -2956,9 +3003,7 @@ describe('NftController', () => { mockGetAccount, triggerSelectedAccountChange, } = setupController({ - options: { - getERC721TokenURI: mockGetERC721TokenURI, - }, + getERC721TokenURI: mockGetERC721TokenURI, }); const firstAddress = '0x123'; @@ -3038,9 +3083,7 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ - options: { - getERC721TokenURI: mockGetERC721TokenURI, - }, + getERC721TokenURI: mockGetERC721TokenURI, }); // Ensure that the currently selected address is not the same as either of the userAddresses triggerSelectedAccountChange(OWNER_ACCOUNT); @@ -3103,7 +3146,6 @@ describe('NftController', () => { describe('removeNft', () => { it('should remove NFT and NFT contract', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -3168,9 +3210,7 @@ describe('NftController', () => { mockGetAccount, triggerSelectedAccountChange, } = setupController({ - options: { - getERC721TokenURI: mockGetERC721TokenURI, - }, + getERC721TokenURI: mockGetERC721TokenURI, }); nock('https://url').get('/').reply(200, { name: 'name', @@ -3228,9 +3268,7 @@ describe('NftController', () => { const tokenURI = 'https://url/'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, changeNetwork } = setupController({ - options: { - getERC721TokenURI: mockGetERC721TokenURI, - }, + getERC721TokenURI: mockGetERC721TokenURI, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -3341,7 +3379,6 @@ describe('NftController', () => { it('should be able to clear the ignoredNfts list', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -3377,10 +3414,8 @@ describe('NftController', () => { .fn() .mockRejectedValue(new Error('ERC1155 error')); const { nftController } = setupController({ - options: { - getERC721OwnerOf: mockGetERC721OwnerOf, - getERC1155BalanceOf: mockGetERC1155BalanceOf, - }, + getERC721OwnerOf: mockGetERC721OwnerOf, + getERC1155BalanceOf: mockGetERC1155BalanceOf, }); const isOwner = await nftController.isNftOwner( @@ -3398,10 +3433,8 @@ describe('NftController', () => { .fn() .mockRejectedValue(new Error('ERC1155 error')); const { nftController } = setupController({ - options: { - getERC721OwnerOf: mockGetERC721OwnerOf, - getERC1155BalanceOf: mockGetERC1155BalanceOf, - }, + getERC721OwnerOf: mockGetERC721OwnerOf, + getERC1155BalanceOf: mockGetERC1155BalanceOf, }); const isOwner = await nftController.isNftOwner( @@ -3418,10 +3451,8 @@ describe('NftController', () => { .fn() .mockRejectedValue(new Error('ERC1155 error')); const { nftController } = setupController({ - options: { - getERC721OwnerOf: mockGetERC721OwnerOf, - getERC1155BalanceOf: mockGetERC1155BalanceOf, - }, + getERC721OwnerOf: mockGetERC721OwnerOf, + getERC1155BalanceOf: mockGetERC1155BalanceOf, }); const isOwner = await nftController.isNftOwner( @@ -3438,10 +3469,8 @@ describe('NftController', () => { .mockRejectedValue(new Error('ERC721 error')); const mockGetERC1155BalanceOf = jest.fn().mockResolvedValue(new BN(1)); const { nftController } = setupController({ - options: { - getERC721OwnerOf: mockGetERC721OwnerOf, - getERC1155BalanceOf: mockGetERC1155BalanceOf, - }, + getERC721OwnerOf: mockGetERC721OwnerOf, + getERC1155BalanceOf: mockGetERC1155BalanceOf, }); const isOwner = await nftController.isNftOwner( @@ -3458,10 +3487,8 @@ describe('NftController', () => { .mockRejectedValue(new Error('ERC721 error')); const mockGetERC1155BalanceOf = jest.fn().mockResolvedValue(new BN(0)); const { nftController } = setupController({ - options: { - getERC721OwnerOf: mockGetERC721OwnerOf, - getERC1155BalanceOf: mockGetERC1155BalanceOf, - }, + getERC721OwnerOf: mockGetERC721OwnerOf, + getERC1155BalanceOf: mockGetERC1155BalanceOf, }); const isOwner = await nftController.isNftOwner( @@ -3481,10 +3508,8 @@ describe('NftController', () => { .fn() .mockRejectedValue(new Error('ERC1155 error')); const { nftController } = setupController({ - options: { - getERC721OwnerOf: mockGetERC721OwnerOf, - getERC1155BalanceOf: mockGetERC1155BalanceOf, - }, + getERC721OwnerOf: mockGetERC721OwnerOf, + getERC1155BalanceOf: mockGetERC1155BalanceOf, }); const error = "Unable to verify ownership. Possibly because the standard is not supported or the user's currently selected network does not match the chain of the asset in question."; @@ -3504,10 +3529,8 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ - options: { - getERC721TokenURI: jest.fn().mockRejectedValue(''), - getERC1155TokenURI: jest.fn().mockResolvedValue('ipfs://*'), - }, + getERC721TokenURI: jest.fn().mockRejectedValue(''), + getERC1155TokenURI: jest.fn().mockResolvedValue('ipfs://*'), defaultSelectedAccount: OWNER_ACCOUNT, }); triggerSelectedAccountChange(OWNER_ACCOUNT); @@ -3538,7 +3561,6 @@ describe('NftController', () => { describe('updateNftFavoriteStatus', () => { it('should not set NFT as favorite if nft not found', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -3566,7 +3588,6 @@ describe('NftController', () => { }); it('should set NFT as favorite', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -3595,7 +3616,6 @@ describe('NftController', () => { it('should set NFT as favorite and then unset it', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -3640,7 +3660,6 @@ describe('NftController', () => { it('should keep the favorite status as true after updating metadata', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -3700,7 +3719,6 @@ describe('NftController', () => { it('should keep the favorite status as false after updating metadata', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -3831,7 +3849,6 @@ describe('NftController', () => { describe('checkAndUpdateAllNftsOwnershipStatus', () => { it('should check whether NFTs for the current selectedAccount/chainId combination are still owned by the selectedAccount and update the isCurrentlyOwned value to false when NFT is not still owned', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); @@ -3860,7 +3877,6 @@ describe('NftController', () => { it('should check whether NFTs for the current selectedAccount/chainId combination are still owned by the selectedAccount and leave/set the isCurrentlyOwned value to true when NFT is still owned', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(true); @@ -3889,7 +3905,6 @@ describe('NftController', () => { it('should check whether NFTs for the current selectedAccount/chainId combination are still owned by the selectedAccount and leave the isCurrentlyOwned value as is when NFT ownership check fails', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); jest @@ -3968,9 +3983,7 @@ describe('NftController', () => { }); it('should handle default case where selectedAccount is not set', async () => { - const { nftController, mockGetAccount } = setupController({ - options: {}, - }); + const { nftController, mockGetAccount } = setupController({}); mockGetAccount.mockReturnValue(null); jest.spyOn(nftController, 'isNftOwner').mockResolvedValue(false); @@ -3994,7 +4007,6 @@ describe('NftController', () => { describe('checkAndUpdateSingleNftOwnershipStatus', () => { it('should check whether the passed NFT is still owned by the the current selectedAccount/chainId combination and update its isCurrentlyOwned property in state if batch is false and isNftOwner returns false', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -4029,7 +4041,6 @@ 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({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -4204,7 +4215,6 @@ describe('NftController', () => { it('should return null if the NFT does not exist in the state', async () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -4292,7 +4302,6 @@ describe('NftController', () => { it('should return undefined if the NFT does not exist', () => { const { nftController } = setupController({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -4326,7 +4335,6 @@ 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({ - options: {}, defaultSelectedAccount: OWNER_ACCOUNT, }); @@ -4375,9 +4383,7 @@ describe('NftController', () => { const tokenURI = 'https://api.pudgypenguins.io/lil/4'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, mockGetAccount } = setupController({ - options: { - getERC721TokenURI: mockGetERC721TokenURI, - }, + getERC721TokenURI: mockGetERC721TokenURI, defaultSelectedAccount: OWNER_ACCOUNT, }); const spy = jest.spyOn(nftController, 'updateNft'); @@ -4432,9 +4438,7 @@ describe('NftController', () => { const tokenURI = 'https://url/'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, mockGetAccount } = setupController({ - options: { - getERC721TokenURI: mockGetERC721TokenURI, - }, + getERC721TokenURI: mockGetERC721TokenURI, defaultSelectedAccount: OWNER_ACCOUNT, }); const updateNftSpy = jest.spyOn(nftController, 'updateNft'); @@ -4498,9 +4502,7 @@ describe('NftController', () => { const tokenURI = 'https://url/'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, mockGetAccount } = setupController({ - options: { - getERC721TokenURI: mockGetERC721TokenURI, - }, + getERC721TokenURI: mockGetERC721TokenURI, defaultSelectedAccount: OWNER_ACCOUNT, }); const spy = jest.spyOn(nftController, 'updateNft'); @@ -4596,9 +4598,7 @@ describe('NftController', () => { changeNetwork, triggerSelectedAccountChange, } = setupController({ - options: { - getERC721TokenURI: mockGetERC721TokenURI, - }, + getERC721TokenURI: mockGetERC721TokenURI, }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); const spy = jest.spyOn(nftController, 'updateNftMetadata'); @@ -4646,9 +4646,7 @@ describe('NftController', () => { changeNetwork, triggerSelectedAccountChange, } = setupController({ - options: { - getERC721TokenURI: mockGetERC721TokenURI, - }, + getERC721TokenURI: mockGetERC721TokenURI, }); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); const spy = jest.spyOn(nftController, 'updateNftMetadata'); @@ -4692,9 +4690,7 @@ describe('NftController', () => { const tokenURI = 'https://api.pudgypenguins.io/lil/4'; const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI); const { nftController, triggerPreferencesStateChange } = setupController({ - options: { - getERC721TokenURI: mockGetERC721TokenURI, - }, + getERC721TokenURI: mockGetERC721TokenURI, }); const selectedAddress = OWNER_ADDRESS; const spy = jest.spyOn(nftController, 'updateNft'); @@ -4799,8 +4795,8 @@ describe('NftController', () => { const { nftController, messenger } = setupController({ options: { openSeaEnabled: true, - getERC721TokenURI: mockGetERC721TokenURI, }, + getERC721TokenURI: mockGetERC721TokenURI, }); const updateNftMetadataSpy = jest.spyOn(nftController, 'updateNftMetadata'); messenger.publish( diff --git a/packages/assets-controllers/src/TokenBalancesController.test.ts b/packages/assets-controllers/src/TokenBalancesController.test.ts index 49390c64fe..e57d13c77f 100644 --- a/packages/assets-controllers/src/TokenBalancesController.test.ts +++ b/packages/assets-controllers/src/TokenBalancesController.test.ts @@ -8,6 +8,8 @@ import { createMockInternalAccount } from '../../accounts-controller/src/tests/m import type { AllowedActions, AllowedEvents, + TokenBalancesControllerActions, + TokenBalancesControllerEvents, TokenBalancesControllerMessenger, } from './TokenBalancesController'; import { TokenBalancesController } from './TokenBalancesController'; @@ -27,13 +29,16 @@ const controllerName = 'TokenBalancesController'; */ function getMessenger( controllerMessenger = new ControllerMessenger< - AllowedActions, - AllowedEvents + TokenBalancesControllerActions | AllowedActions, + TokenBalancesControllerEvents | AllowedEvents >(), ): TokenBalancesControllerMessenger { return controllerMessenger.getRestricted({ name: controllerName, - allowedActions: ['AccountsController:getSelectedAccount'], + allowedActions: [ + 'AccountsController:getSelectedAccount', + 'AssetsContractController:getERC20BalanceOf', + ], allowedEvents: ['TokensController:stateChange'], }); } @@ -55,8 +60,8 @@ const setupController = ({ triggerTokensStateChange: (state: TokensControllerState) => Promise; } => { const controllerMessenger = new ControllerMessenger< - AllowedActions, - AllowedEvents + TokenBalancesControllerActions | AllowedActions, + TokenBalancesControllerEvents | AllowedEvents >(); const messenger = getMessenger(controllerMessenger); @@ -67,9 +72,12 @@ const setupController = ({ 'AccountsController:getSelectedAccount', mockSelectedAccount, ); + controllerMessenger.registerActionHandler( + 'AssetsContractController:getERC20BalanceOf', + mockGetERC20BalanceOf, + ); const controller = new TokenBalancesController({ - getERC20BalanceOf: mockGetERC20BalanceOf, messenger, ...config, }); @@ -114,7 +122,6 @@ describe('TokenBalancesController', () => { new TokenBalancesController({ interval: 10, - getERC20BalanceOf: jest.fn(), messenger: getMessenger(new ControllerMessenger()), }); await flushPromises(); From 39ac871fc06269074a1bbb9ff144d5a560c7a4a0 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 2 Jul 2024 18:25:53 -0400 Subject: [PATCH 15/40] Pass callbacks into `expect` --- ...tractControllerWithNetworkClientId.test.ts | 120 ++++++++++-------- 1 file changed, 66 insertions(+), 54 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts b/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts index 6daeafeae6..dd0d9c6c63 100644 --- a/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts +++ b/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts @@ -21,25 +21,31 @@ describe('AssetsContractController with NetworkClientId', () => { it('should throw when getting ERC-20 token balance when networkClientId is invalid', async () => { const { messenger } = await setupAssetContractControllers(); await expect( - messenger.call( - `AssetsContractController:getERC20BalanceOf`, - ERC20_UNI_ADDRESS, - TEST_ACCOUNT_PUBLIC_ADDRESS, - 'invalidNetworkClientId', - ), - ).rejects.toThrow('No custom network client was found'); + async () => + await messenger.call( + `AssetsContractController:getERC20BalanceOf`, + ERC20_UNI_ADDRESS, + TEST_ACCOUNT_PUBLIC_ADDRESS, + 'invalidNetworkClientId', + ), + ).rejects.toThrow( + `No custom network client was found with the ID "invalidNetworkClientId".`, + ); messenger.clearEventSubscriptions('NetworkController:stateChange'); }); it('should throw when getting ERC-20 token decimal when networkClientId is invalid', async () => { const { messenger } = await setupAssetContractControllers(); await expect( - messenger.call( - `AssetsContractController:getERC20TokenDecimals`, - ERC20_UNI_ADDRESS, - 'invalidNetworkClientId', - ), - ).rejects.toThrow('No custom network client was found'); + async () => + await messenger.call( + `AssetsContractController:getERC20TokenDecimals`, + ERC20_UNI_ADDRESS, + 'invalidNetworkClientId', + ), + ).rejects.toThrow( + `No custom network client was found with the ID "invalidNetworkClientId".`, + ); messenger.clearEventSubscriptions('NetworkController:stateChange'); }); @@ -138,13 +144,14 @@ describe('AssetsContractController with NetworkClientId', () => { it('should throw error when getting ERC-721 token standard and details when networkClientId is invalid', async () => { const { messenger } = await setupAssetContractControllers(); await expect( - messenger.call( - `AssetsContractController:getTokenStandardAndDetails`, - ERC20_UNI_ADDRESS, - TEST_ACCOUNT_PUBLIC_ADDRESS, - undefined, - 'invalidNetworkClientId', - ), + async () => + await messenger.call( + `AssetsContractController:getTokenStandardAndDetails`, + ERC20_UNI_ADDRESS, + TEST_ACCOUNT_PUBLIC_ADDRESS, + undefined, + 'invalidNetworkClientId', + ), ).rejects.toThrow('No custom network client was found'); messenger.clearEventSubscriptions('NetworkController:stateChange'); }); @@ -153,13 +160,14 @@ describe('AssetsContractController with NetworkClientId', () => { const { messenger } = await setupAssetContractControllers(); const error = 'Unable to determine contract standard'; await expect( - messenger.call( - `AssetsContractController:getTokenStandardAndDetails`, - 'BaDeRc20AdDrEsS', - TEST_ACCOUNT_PUBLIC_ADDRESS, - undefined, - 'mainnet', - ), + async () => + await messenger.call( + `AssetsContractController:getTokenStandardAndDetails`, + 'BaDeRc20AdDrEsS', + TEST_ACCOUNT_PUBLIC_ADDRESS, + undefined, + 'mainnet', + ), ).rejects.toThrow(error); messenger.clearEventSubscriptions('NetworkController:stateChange'); }); @@ -558,11 +566,12 @@ describe('AssetsContractController with NetworkClientId', () => { it('should throw error when getting ERC-721 NFT symbol when networkClientId is invalid', async () => { const { messenger } = await setupAssetContractControllers(); await expect( - messenger.call( - `AssetsContractController:getERC721AssetSymbol`, - ERC721_GODS_ADDRESS, - 'invalidNetworkClientId', - ), + async () => + await messenger.call( + `AssetsContractController:getERC721AssetSymbol`, + ERC721_GODS_ADDRESS, + 'invalidNetworkClientId', + ), ).rejects.toThrow('No custom network client was found'); messenger.clearEventSubscriptions('NetworkController:stateChange'); }); @@ -672,12 +681,13 @@ describe('AssetsContractController with NetworkClientId', () => { it('should throw error when getting ERC-721 NFT ownership using networkClientId that is invalid', async () => { const { messenger } = await setupAssetContractControllers(); await expect( - messenger.call( - `AssetsContractController:getERC721OwnerOf`, - ERC721_GODS_ADDRESS, - '148332', - 'invalidNetworkClientId', - ), + async () => + await messenger.call( + `AssetsContractController:getERC721OwnerOf`, + ERC721_GODS_ADDRESS, + '148332', + 'invalidNetworkClientId', + ), ).rejects.toThrow('No custom network client was found'); messenger.clearEventSubscriptions('NetworkController:stateChange'); }); @@ -792,15 +802,16 @@ describe('AssetsContractController with NetworkClientId', () => { it('should throw error when transferring single ERC-1155 when networkClientId is invalid', async () => { const { messenger } = await setupAssetContractControllers(); await expect( - messenger.call( - `AssetsContractController:transferSingleERC1155`, - ERC1155_ADDRESS, - TEST_ACCOUNT_PUBLIC_ADDRESS, - TEST_ACCOUNT_PUBLIC_ADDRESS, - ERC1155_ID, - '1', - 'invalidNetworkClientId', - ), + async () => + await messenger.call( + `AssetsContractController:transferSingleERC1155`, + ERC1155_ADDRESS, + TEST_ACCOUNT_PUBLIC_ADDRESS, + TEST_ACCOUNT_PUBLIC_ADDRESS, + ERC1155_ID, + '1', + 'invalidNetworkClientId', + ), ).rejects.toThrow('No custom network client was found'); messenger.clearEventSubscriptions('NetworkController:stateChange'); }); @@ -843,13 +854,14 @@ describe('AssetsContractController with NetworkClientId', () => { it('should throw error when getting the balance of a ERC-1155 NFT when networkClientId is invalid', async () => { const { messenger } = await setupAssetContractControllers(); await expect( - messenger.call( - `AssetsContractController:getERC1155BalanceOf`, - TEST_ACCOUNT_PUBLIC_ADDRESS, - ERC1155_ADDRESS, - ERC1155_ID, - 'invalidNetworkClientId', - ), + async () => + await messenger.call( + `AssetsContractController:getERC1155BalanceOf`, + TEST_ACCOUNT_PUBLIC_ADDRESS, + ERC1155_ADDRESS, + ERC1155_ID, + 'invalidNetworkClientId', + ), ).rejects.toThrow('No custom network client was found'); messenger.clearEventSubscriptions('NetworkController:stateChange'); }); From 3964cb108fe03e5907b4c58490d0f11a19b5e4ce Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 11 Jul 2024 16:29:59 -0400 Subject: [PATCH 16/40] Fix `setupAssetContractControllers` so that `useNetworkControllerProvider` option works --- .../src/AssetsContractController.test.ts | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.test.ts b/packages/assets-controllers/src/AssetsContractController.test.ts index 28668b5354..a593b2e9a8 100644 --- a/packages/assets-controllers/src/AssetsContractController.test.ts +++ b/packages/assets-controllers/src/AssetsContractController.test.ts @@ -101,26 +101,28 @@ async function setupAssetContractControllers({ const selectedNetworkClient = networkController.getSelectedNetworkClient(); assert(selectedNetworkClient, 'No network is selected'); provider = selectedNetworkClient.provider; - - controllerMessenger.unregisterActionHandler( - 'NetworkController:getNetworkClientById', - ); - controllerMessenger.registerActionHandler( - 'NetworkController:getNetworkClientById', - // @ts-expect-error TODO: remove this annotation once the `Eip1193Provider` class is released - (networkClientId: NetworkClientId) => { - return { - ...networkController.getNetworkClientById(networkClientId), - provider, - }; - }, - ); } else { provider = new HttpProvider( `https://mainnet.infura.io/v3/${infuraProjectId}`, ); } + controllerMessenger.unregisterActionHandler( + 'NetworkController:getNetworkClientById', + ); + controllerMessenger.registerActionHandler( + 'NetworkController:getNetworkClientById', + // @ts-expect-error TODO: remove this annotation once the `Eip1193Provider` class is released + (networkClientId: NetworkClientId) => { + return useNetworkControllerProvider + ? networkController.getNetworkClientById.bind(networkController) + : { + ...networkController.getNetworkClientById(networkClientId), + provider, + }; + }, + ); + const assetsContractMessenger = controllerMessenger.getRestricted({ name: 'AssetsContractController', allowedActions: ['NetworkController:getNetworkClientById'], @@ -1193,9 +1195,8 @@ describe('AssetsContractController', () => { }); it('should get the balance of a ERC-1155 NFT for a given address', async () => { - const { assetsContract, messenger, provider, networkClientConfiguration } = + const { messenger, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -1223,7 +1224,7 @@ describe('AssetsContractController', () => { ERC1155_ADDRESS, ERC1155_ID, ); - expect(balance.isZero() || balance.isNeg()).toBe(false); + expect(Number(balance)).toBeGreaterThan(0); messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); From 079f4cd737941d41b2b1dcfde7c80ca0b3cccf50 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 11 Jul 2024 16:39:39 -0400 Subject: [PATCH 17/40] Make class fields accessible but immutable by converting to private but exposing getter --- .../src/AssetsContractController.ts | 47 ++++++++++++------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 126d3d17aa..85dc782083 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -169,9 +169,9 @@ export class AssetsContractController { #provider: Provider | undefined; - ipfsGateway: string; + #ipfsGateway: string; - chainId: Hex; + #chainId: Hex; /** * Creates a AssetsContractController instance. @@ -189,8 +189,8 @@ export class AssetsContractController { }) { this.messagingSystem = messenger; this.#provider = undefined; - this.ipfsGateway = IPFS_DEFAULT_GATEWAY_URL; - this.chainId = initialChainId; + this.#ipfsGateway = IPFS_DEFAULT_GATEWAY_URL; + this.#chainId = initialChainId; this.#registerActionHandlers(); this.#registerEventSubscriptions(); @@ -198,18 +198,21 @@ export class AssetsContractController { // TODO: Expand into base-controller utility function that batch registers action handlers. #registerActionHandlers() { + const nonMethodClassProperties = [ + 'constructor', + 'messagingSystem', + 'provider', + 'ipfsGateway', + 'chainId', + ]; + for (const method of Object.getOwnPropertyNames( Object.getPrototypeOf(this), ) as (keyof this)[]) { if ( ((key: keyof this): key is AssetsContractControllerMethodName => - ![ - 'constructor', - 'messagingSystem', - 'provider', - 'ipfsGateway', - 'chainId', - ].find((e) => e === key) && typeof this[key] === 'function')(method) + !nonMethodClassProperties.find((e) => e === key) && + typeof this[key] === 'function')(method) ) { this.messagingSystem.registerActionHandler( `${name}:${method}`, @@ -225,7 +228,7 @@ export class AssetsContractController { this.messagingSystem.subscribe( `PreferencesController:stateChange`, ({ ipfsGateway }) => { - this.ipfsGateway = ipfsGateway; + this.#ipfsGateway = ipfsGateway; }, ); @@ -239,8 +242,8 @@ export class AssetsContractController { selectedNetworkClientId, ); - if (this.chainId !== chainId) { - this.chainId = chainId; + if (this.#chainId !== chainId) { + this.#chainId = chainId; // @ts-expect-error TODO: remove this annotation once the `Eip1193Provider` class is released this.#provider = this.#getCorrectProvider(); } @@ -263,6 +266,14 @@ export class AssetsContractController { throw new Error('Property only used for setting'); } + get ipfsGateway() { + return this.#ipfsGateway; + } + + get chainId() { + return this.#chainId; + } + /** * Get the relevant provider instance. * @@ -296,7 +307,7 @@ export class AssetsContractController { `NetworkController:getNetworkClientById`, networkClientId, ).configuration.chainId - : this.chainId; + : this.#chainId; } /** @@ -429,7 +440,7 @@ export class AssetsContractController { return { ...(await erc721Standard.getDetails( tokenAddress, - this.ipfsGateway, + this.#ipfsGateway, tokenId, )), }; @@ -443,7 +454,7 @@ export class AssetsContractController { return { ...(await erc1155Standard.getDetails( tokenAddress, - this.ipfsGateway, + this.#ipfsGateway, tokenId, )), }; @@ -630,7 +641,7 @@ export class AssetsContractController { tokensToDetect.forEach((tokenAddress, index) => { const balance: BN = result[index]; /* istanbul ignore else */ - if (String(balance) !== '0') { + if (!balance.isZero()) { nonZeroBalances[tokenAddress] = balance; } }); From f1f58e77b4d3e7e8666374b19826b53754d77b82 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 11 Jul 2024 16:41:57 -0400 Subject: [PATCH 18/40] fix `NftController` test mock action handlers to be excluded from `options` bag --- .../src/NftController.test.ts | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 327c057c90..17bc70c349 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -1847,15 +1847,13 @@ describe('NftController', () => { }); it('should add NFT erc721 and aggregate NFT data from both contract and NFT-API when call to Get Collections succeeds', async () => { const { nftController } = setupController({ - options: { - getERC721AssetName: jest.fn().mockResolvedValue('KudosToken'), - getERC721AssetSymbol: jest.fn().mockResolvedValue('KDO'), - getERC721TokenURI: jest - .fn() - .mockResolvedValue( - 'https://ipfs.gitcoin.co:443/api/v0/cat/QmPmt6EAaioN78ECnW5oCL8v2YvVSpoBjLCjrXhhsAvoov', - ), - }, + getERC721AssetName: jest.fn().mockResolvedValue('KudosToken'), + getERC721AssetSymbol: jest.fn().mockResolvedValue('KDO'), + getERC721TokenURI: jest + .fn() + .mockResolvedValue( + 'https://ipfs.gitcoin.co:443/api/v0/cat/QmPmt6EAaioN78ECnW5oCL8v2YvVSpoBjLCjrXhhsAvoov', + ), defaultSelectedAccount: OWNER_ACCOUNT, }); nock(NFT_API_BASE_URL) @@ -2353,13 +2351,13 @@ describe('NftController', () => { const { nftController } = setupController({ options: { onNftAdded: mockOnNftAdded, - getERC721AssetName: jest - .fn() - .mockRejectedValue(new Error('Failed to fetch')), - getERC721AssetSymbol: jest - .fn() - .mockRejectedValue(new Error('Failed to fetch')), }, + getERC721AssetName: jest + .fn() + .mockRejectedValue(new Error('Failed to fetch')), + getERC721AssetSymbol: jest + .fn() + .mockRejectedValue(new Error('Failed to fetch')), defaultSelectedAccount: OWNER_ACCOUNT, }); nock(NFT_API_BASE_URL) From 2de5258a78ee25926fe1c73335bf291f082ed858 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Sun, 14 Jul 2024 00:41:05 -0400 Subject: [PATCH 19/40] Replace references to `NetworkController:stateChange` event with `NetworkController:networkDidChange` event --- ...tractControllerWithNetworkClientId.test.ts | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts b/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts index dd0d9c6c63..97760db63f 100644 --- a/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts +++ b/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts @@ -31,7 +31,7 @@ describe('AssetsContractController with NetworkClientId', () => { ).rejects.toThrow( `No custom network client was found with the ID "invalidNetworkClientId".`, ); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should throw when getting ERC-20 token decimal when networkClientId is invalid', async () => { @@ -46,7 +46,7 @@ describe('AssetsContractController with NetworkClientId', () => { ).rejects.toThrow( `No custom network client was found with the ID "invalidNetworkClientId".`, ); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get balance of ERC-20 token contract correctly', async () => { @@ -103,7 +103,7 @@ describe('AssetsContractController with NetworkClientId', () => { ); expect(UNIBalance.toString(16)).not.toBe('0'); expect(UNINoBalance.toString(16)).toBe('0'); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get ERC-721 NFT tokenId correctly', async () => { @@ -138,7 +138,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'mainnet', ); expect(tokenId).not.toBe(0); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should throw error when getting ERC-721 token standard and details when networkClientId is invalid', async () => { @@ -153,7 +153,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'invalidNetworkClientId', ), ).rejects.toThrow('No custom network client was found'); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should throw contract standard error when getting ERC-20 token standard and details when provided with invalid ERC-20 address', async () => { @@ -169,7 +169,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'mainnet', ), ).rejects.toThrow(error); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get ERC-721 token standard and details', async () => { @@ -236,7 +236,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'mainnet', ); expect(standardAndDetails.standard).toBe('ERC721'); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get ERC-1155 token standard and details', async () => { @@ -287,7 +287,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'mainnet', ); expect(standardAndDetails.standard).toBe('ERC1155'); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get ERC-20 token standard and details', async () => { @@ -386,7 +386,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'mainnet', ); expect(standardAndDetails.standard).toBe('ERC20'); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get ERC-721 NFT tokenURI correctly', async () => { @@ -436,7 +436,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'mainnet', ); expect(tokenId).toBe('https://api.godsunchained.com/card/0'); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should not throw an error when address given is does not support NFT Metadata interface', async () => { @@ -494,7 +494,7 @@ describe('AssetsContractController with NetworkClientId', () => { expect(errorLogSpy.mock.calls).toContainEqual([ 'Contract does not support ERC721 metadata interface.', ]); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get ERC-721 NFT name', async () => { @@ -527,7 +527,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'mainnet', ); expect(name).toBe('Gods Unchained'); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get ERC-721 NFT symbol', async () => { @@ -560,7 +560,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'mainnet', ); expect(symbol).toBe('GODS'); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should throw error when getting ERC-721 NFT symbol when networkClientId is invalid', async () => { @@ -573,7 +573,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'invalidNetworkClientId', ), ).rejects.toThrow('No custom network client was found'); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get ERC-20 token decimals', async () => { @@ -606,7 +606,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'mainnet', ); expect(Number(decimals)).toBe(18); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get ERC-20 token name', async () => { @@ -641,7 +641,7 @@ describe('AssetsContractController with NetworkClientId', () => { ); expect(name).toBe('Dai Stablecoin'); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get ERC-721 NFT ownership', async () => { @@ -675,7 +675,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'mainnet', ); expect(tokenId).not.toBe(''); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should throw error when getting ERC-721 NFT ownership using networkClientId that is invalid', async () => { @@ -689,7 +689,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'invalidNetworkClientId', ), ).rejects.toThrow('No custom network client was found'); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get balance of ERC-20 token in a single call on network with token detection support', async () => { @@ -723,7 +723,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'mainnet', ); expect(balances[ERC20_SAI_ADDRESS]).toBeDefined(); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should not have balance in a single call after switching to network without token detection support', async () => { @@ -796,7 +796,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'sepolia', ); expect(noBalances).toStrictEqual({}); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should throw error when transferring single ERC-1155 when networkClientId is invalid', async () => { @@ -813,7 +813,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'invalidNetworkClientId', ), ).rejects.toThrow('No custom network client was found'); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get the balance of a ERC-1155 NFT for a given address', async () => { @@ -848,7 +848,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'mainnet', ); expect(Number(balance)).toBeGreaterThan(0); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should throw error when getting the balance of a ERC-1155 NFT when networkClientId is invalid', async () => { @@ -863,7 +863,7 @@ describe('AssetsContractController with NetworkClientId', () => { 'invalidNetworkClientId', ), ).rejects.toThrow('No custom network client was found'); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should get the URI of a ERC-1155 NFT', async () => { @@ -898,6 +898,6 @@ describe('AssetsContractController with NetworkClientId', () => { 'mainnet', ); expect(uri.toLowerCase()).toStrictEqual(expectedUri); - messenger.clearEventSubscriptions('NetworkController:stateChange'); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); }); From 69ba701a2e89f66132814b2ff4212afc94573057 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 15 Jul 2024 20:23:46 -0400 Subject: [PATCH 20/40] Fix `#getCorrect{Provider,ChainId}` methods so that the methods return up-to-date results by referencing the currently selected network in the network-controller, even if the `networkClientId` param is omitted. --- .../src/AssetsContractController.test.ts | 7 ++- .../src/AssetsContractController.ts | 44 ++++++++++++------- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.test.ts b/packages/assets-controllers/src/AssetsContractController.test.ts index a593b2e9a8..217d36f8ef 100644 --- a/packages/assets-controllers/src/AssetsContractController.test.ts +++ b/packages/assets-controllers/src/AssetsContractController.test.ts @@ -125,7 +125,12 @@ async function setupAssetContractControllers({ const assetsContractMessenger = controllerMessenger.getRestricted({ name: 'AssetsContractController', - allowedActions: ['NetworkController:getNetworkClientById'], + allowedActions: [ + 'NetworkController:getNetworkClientById', + 'NetworkController:getNetworkConfigurationByNetworkClientId', + 'NetworkController:getSelectedNetworkClient', + 'NetworkController:getState', + ], allowedEvents: [ 'PreferencesController:stateChange', 'NetworkController:networkDidChange', diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 85dc782083..7b32ffa93e 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -8,6 +8,9 @@ import { IPFS_DEFAULT_GATEWAY_URL } from '@metamask/controller-utils'; import type { NetworkClientId, NetworkControllerGetNetworkClientByIdAction, + NetworkControllerGetNetworkConfigurationByNetworkClientId, + NetworkControllerGetSelectedNetworkClientAction, + NetworkControllerGetStateAction, NetworkControllerNetworkDidChangeEvent, Provider, } from '@metamask/network-controller'; @@ -147,7 +150,11 @@ export type AssetsContractControllerGetBalancesInSingleCallAction = export type AssetsContractControllerEvents = never; -export type AllowedActions = NetworkControllerGetNetworkClientByIdAction; +export type AllowedActions = + | NetworkControllerGetNetworkClientByIdAction + | NetworkControllerGetNetworkConfigurationByNetworkClientId + | NetworkControllerGetSelectedNetworkClientAction + | NetworkControllerGetStateAction; export type AllowedEvents = | PreferencesControllerStateChangeEvent @@ -235,12 +242,7 @@ export class AssetsContractController { this.messagingSystem.subscribe( `NetworkController:networkDidChange`, ({ selectedNetworkClientId }) => { - const { - configuration: { chainId }, - } = this.messagingSystem.call( - `NetworkController:getNetworkClientById`, - selectedNetworkClientId, - ); + const chainId = this.#getCorrectChainId(selectedNetworkClientId); if (this.#chainId !== chainId) { this.#chainId = chainId; @@ -286,7 +288,8 @@ export class AssetsContractController { `NetworkController:getNetworkClientById`, networkClientId, ).provider - : this.#provider; + : this.messagingSystem.call('NetworkController:getSelectedNetworkClient') + ?.provider ?? this.#provider; if (provider === undefined) { throw new Error(MISSING_PROVIDER_ERROR); @@ -302,12 +305,23 @@ export class AssetsContractController { * @returns Hex chain ID. */ #getCorrectChainId(networkClientId?: NetworkClientId): Hex { - return networkClientId - ? this.messagingSystem.call( - `NetworkController:getNetworkClientById`, - networkClientId, - ).configuration.chainId - : this.#chainId; + if (networkClientId) { + const networkClientConfiguration = this.messagingSystem.call( + 'NetworkController:getNetworkConfigurationByNetworkClientId', + networkClientId, + ); + if (networkClientConfiguration) { + return networkClientConfiguration.chainId; + } + } + const { selectedNetworkClientId } = this.messagingSystem.call( + 'NetworkController:getState', + ); + const networkClient = this.messagingSystem.call( + 'NetworkController:getNetworkClientById', + selectedNetworkClientId, + ); + return networkClient.configuration?.chainId ?? this.#chainId; } /** @@ -641,7 +655,7 @@ export class AssetsContractController { tokensToDetect.forEach((tokenAddress, index) => { const balance: BN = result[index]; /* istanbul ignore else */ - if (!balance.isZero()) { + if (String(balance) !== '0') { nonZeroBalances[tokenAddress] = balance; } }); From a258fa2a6792570c2673494145fe8a6dedf1ad10 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 15 Jul 2024 20:24:27 -0400 Subject: [PATCH 21/40] Fix `setupAssetContractControllers`, revert unintended changes to `useNetworkControllerProvider` flag behavior --- .../src/AssetsContractController.test.ts | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.test.ts b/packages/assets-controllers/src/AssetsContractController.test.ts index 217d36f8ef..194628006a 100644 --- a/packages/assets-controllers/src/AssetsContractController.test.ts +++ b/packages/assets-controllers/src/AssetsContractController.test.ts @@ -11,13 +11,14 @@ import HttpProvider from '@metamask/ethjs-provider-http'; import type { Provider, NetworkClientId, - NetworkControllerEvents, NetworkControllerActions, + NetworkControllerEvents, } from '@metamask/network-controller'; import { NetworkController, NetworkClientType, } from '@metamask/network-controller'; +import type { PreferencesState } from '@metamask/preferences-controller'; import { getDefaultPreferencesState } from '@metamask/preferences-controller'; import assert from 'assert'; @@ -60,7 +61,7 @@ const TEST_ACCOUNT_PUBLIC_ADDRESS = */ async function setupAssetContractControllers({ options, - useNetworkControllerProvider, + useNetworkControllerProvider = false, infuraProjectId = '341eacb578dd44a1a049cbc5f6fd4035', }: { options?: Partial< @@ -86,14 +87,13 @@ async function setupAssetContractControllers({ | AssetsContractControllerEvents | AssetsContractAllowedEvents >(); - const networkMessenger = controllerMessenger.getRestricted({ - name: 'NetworkController', - allowedActions: [], - allowedEvents: [], - }); const networkController = new NetworkController({ infuraProjectId, - messenger: networkMessenger, + messenger: controllerMessenger.getRestricted({ + name: 'NetworkController', + allowedActions: [], + allowedEvents: [], + }), trackMetaMetricsEvent: jest.fn(), }); if (useNetworkControllerProvider) { @@ -113,14 +113,12 @@ async function setupAssetContractControllers({ controllerMessenger.registerActionHandler( 'NetworkController:getNetworkClientById', // @ts-expect-error TODO: remove this annotation once the `Eip1193Provider` class is released - (networkClientId: NetworkClientId) => { - return useNetworkControllerProvider - ? networkController.getNetworkClientById.bind(networkController) - : { - ...networkController.getNetworkClientById(networkClientId), - provider, - }; - }, + useNetworkControllerProvider + ? networkController.getNetworkClientById.bind(networkController) + : (networkClientId: NetworkClientId) => ({ + ...networkController.getNetworkClientById(networkClientId), + provider, + }), ); const assetsContractMessenger = controllerMessenger.getRestricted({ @@ -149,6 +147,13 @@ async function setupAssetContractControllers({ provider, networkClientConfiguration, infuraProjectId, + triggerPreferencesStateChange: (state: PreferencesState) => { + controllerMessenger.publish( + 'PreferencesController:stateChange', + state, + [], + ); + }, }; } From bbb0b0fffed618d88623c1a6286f5bbcd4187ddc Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 15 Jul 2024 20:25:01 -0400 Subject: [PATCH 22/40] Clean up `AssetsContractController` tests --- .../src/AssetsContractController.test.ts | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.test.ts b/packages/assets-controllers/src/AssetsContractController.test.ts index 194628006a..e7c1d81daa 100644 --- a/packages/assets-controllers/src/AssetsContractController.test.ts +++ b/packages/assets-controllers/src/AssetsContractController.test.ts @@ -206,7 +206,8 @@ describe('AssetsContractController', () => { }); it('should update the ipfsGateWay config value when this value is changed in the preferences controller', async () => { - const { assetsContract, messenger } = await setupAssetContractControllers(); + const { assetsContract, messenger, triggerPreferencesStateChange } = + await setupAssetContractControllers(); expect({ chainId: assetsContract.chainId, ipfsGateway: assetsContract.ipfsGateway, @@ -215,14 +216,10 @@ describe('AssetsContractController', () => { ipfsGateway: IPFS_DEFAULT_GATEWAY_URL, }); - messenger.publish( - 'PreferencesController:stateChange', - { - ...getDefaultPreferencesState(), - ipfsGateway: 'newIPFSGateWay', - }, - [], - ); + triggerPreferencesStateChange({ + ...getDefaultPreferencesState(), + ipfsGateway: 'newIPFSGateWay', + }); expect({ chainId: assetsContract.chainId, @@ -1039,7 +1036,7 @@ describe('AssetsContractController', () => { }, ], }); - const { assetsContract, messenger, network, provider } = + const { assetsContract, messenger, provider } = await setupAssetContractControllers({ options: { chainId: ChainId.mainnet, @@ -1050,7 +1047,7 @@ describe('AssetsContractController', () => { assetsContract.provider = provider; const balancesOnMainnet = await messenger.call( - `AssetsContractController:getBalancesInSingleCall`, + 'AssetsContractController:getBalancesInSingleCall', ERC20_SAI_ADDRESS, [ERC20_SAI_ADDRESS], ); @@ -1058,17 +1055,20 @@ describe('AssetsContractController', () => { [ERC20_SAI_ADDRESS]: BigNumber.from('0x0733ed8ef4c4a0155d09'), }); - await network.setActiveNetwork(InfuraNetworkType['linea-mainnet']); + await messenger.call( + `NetworkController:setActiveNetwork`, + InfuraNetworkType['linea-mainnet'], + ); const balancesOnLineaMainnet = await messenger.call( - `AssetsContractController:getBalancesInSingleCall`, + 'AssetsContractController:getBalancesInSingleCall', ERC20_SAI_ADDRESS, [ERC20_SAI_ADDRESS], - InfuraNetworkType['linea-mainnet'], ); expect(balancesOnLineaMainnet).toStrictEqual({ [ERC20_SAI_ADDRESS]: BigNumber.from('0xa0155d09733ed8ef4c4'), }); + messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); it('should not have balance in a single call after switching to network without token detection support', async () => { @@ -1205,8 +1205,9 @@ describe('AssetsContractController', () => { }); it('should get the balance of a ERC-1155 NFT for a given address', async () => { - const { messenger, networkClientConfiguration } = + const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); + assetsContract.provider = provider; mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ From 30d2f293904d07f5f8fa5779e7c2b26993697076 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 15 Jul 2024 21:40:14 -0400 Subject: [PATCH 23/40] Fix `BN.isZero()` call that throws error, causing ERC-1155 ownership check to return exception --- 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 7ea8328567..949041f651 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -1420,7 +1420,7 @@ export class NftController extends BaseController< tokenId, networkClientId, ); - return !balance.isZero(); + return String(balance) !== '0'; // eslint-disable-next-line no-empty } catch { // Ignore ERC-1155 contract error From 38fb163dd3ca1d740c186da8d31998d7c0ca703d Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 15 Jul 2024 21:42:13 -0400 Subject: [PATCH 24/40] Fix `NftController` `watchNft` test errors caused by changes to `messenger.call` invocation order --- .../src/NftController.test.ts | 78 ++++++++++--------- 1 file changed, 41 insertions(+), 37 deletions(-) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 17bc70c349..1faa90bee9 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -54,12 +54,10 @@ import type { Nft, NftControllerState, NftControllerMessenger, + AllowedActions as NftControllerAllowedActions, + AllowedEvents as NftControllerAllowedEvents, } from './NftController'; -import { - NftController, - type AllowedActions, - type AllowedEvents, -} from './NftController'; +import { NftController } from './NftController'; const CRYPTOPUNK_ADDRESS = '0xb47e3cd837dDF8e4c57F05d70Ab865de6e193BBB'; const ERC721_KUDOSADDRESS = '0x2aEa4Add166EBf38b63d09a75dE1a7b94Aa24163'; @@ -195,10 +193,10 @@ function setupController({ } = {}) { const messenger = new ControllerMessenger< | ExtractAvailableAction - | AllowedActions + | NftControllerAllowedActions | ExtractAvailableAction, | ExtractAvailableEvent - | AllowedEvents + | NftControllerAllowedEvents | ExtractAvailableEvent | AccountsControllerSelectedAccountChangeEvent >(); @@ -660,14 +658,15 @@ describe('NftController', () => { const callActionSpy = jest .spyOn(messenger, 'call') .mockReturnValueOnce(OWNER_ACCOUNT) - .mockResolvedValueOnce({}) - .mockReturnValueOnce(OWNER_ACCOUNT); + .mockResolvedValue({}); await nftController.watchNft(ERC721_NFT, ERC721, 'https://test-dapp.com'); - // First call is getInternalAccount. Second call is the approval request. - expect(callActionSpy).toHaveBeenCalledTimes(3); + // First call is `AccountsController:getAccount` + // Fifth call is `ApprovalController:addRequest` + // Sixth call is `AccountsController:getAccount` + expect(callActionSpy).toHaveBeenCalledTimes(6); expect(callActionSpy).toHaveBeenNthCalledWith( - 2, + 5, 'ApprovalController:addRequest', { id: requestId, @@ -729,14 +728,15 @@ describe('NftController', () => { const callActionSpy = jest .spyOn(messenger, 'call') .mockReturnValueOnce(OWNER_ACCOUNT) - .mockResolvedValueOnce({}) - .mockReturnValueOnce(OWNER_ACCOUNT); + .mockResolvedValue({}); await nftController.watchNft(ERC721_NFT, ERC721, 'https://test-dapp.com'); - // First call is getInternalAccount. Second call is the approval request. - expect(callActionSpy).toHaveBeenCalledTimes(3); + // First call is `AccountsController:getAccount` + // Fifth call is `ApprovalController:addRequest` + // Sixth call is `AccountsController:getAccount` + expect(callActionSpy).toHaveBeenCalledTimes(6); expect(callActionSpy).toHaveBeenNthCalledWith( - 2, + 5, 'ApprovalController:addRequest', { id: requestId, @@ -798,14 +798,15 @@ describe('NftController', () => { const callActionSpy = jest .spyOn(messenger, 'call') .mockReturnValueOnce(OWNER_ACCOUNT) - .mockResolvedValueOnce({}) - .mockReturnValueOnce(OWNER_ACCOUNT); + .mockResolvedValue({}); await nftController.watchNft(ERC721_NFT, ERC721, 'https://test-dapp.com'); - // First call is getInternalAccount. Second call is the approval request. - expect(callActionSpy).toHaveBeenCalledTimes(3); + // First call is `AccountsController:getAccount` + // Fifth call is `ApprovalController:addRequest` + // Sixth call is `AccountsController:getAccount` + expect(callActionSpy).toHaveBeenCalledTimes(6); expect(callActionSpy).toHaveBeenNthCalledWith( - 2, + 5, 'ApprovalController:addRequest', { id: requestId, @@ -868,14 +869,15 @@ describe('NftController', () => { const callActionSpy = jest .spyOn(messenger, 'call') .mockReturnValueOnce(OWNER_ACCOUNT) - .mockResolvedValueOnce({}) - .mockReturnValueOnce(OWNER_ACCOUNT); + .mockResolvedValue({}); await nftController.watchNft(ERC721_NFT, ERC721, 'https://test-dapp.com'); - // First call is getInternalAccount. Second call is the approval request. - expect(callActionSpy).toHaveBeenCalledTimes(3); + // First call is `AccountsController:getAccount` + // Fifth call is `ApprovalController:addRequest` + // Sixth call is `AccountsController:getAccount` + expect(callActionSpy).toHaveBeenCalledTimes(6); expect(callActionSpy).toHaveBeenNthCalledWith( - 2, + 5, 'ApprovalController:addRequest', { id: requestId, @@ -941,18 +943,19 @@ describe('NftController', () => { const callActionSpy = jest .spyOn(messenger, 'call') .mockReturnValueOnce(OWNER_ACCOUNT) - .mockResolvedValueOnce({}) - .mockReturnValueOnce(OWNER_ACCOUNT); + .mockResolvedValue({}); await nftController.watchNft( ERC1155_NFT, ERC1155, 'https://etherscan.io', ); - // First call is getInternalAccount. Second call is the approval request. - expect(callActionSpy).toHaveBeenCalledTimes(3); + // First call is `AccountsController:getAccount` + // Fifth call is `ApprovalController:addRequest` + // Sixth call is `AccountsController:getAccount` + expect(callActionSpy).toHaveBeenCalledTimes(6); expect(callActionSpy).toHaveBeenNthCalledWith( - 2, + 5, 'ApprovalController:addRequest', { id: requestId, @@ -1012,18 +1015,19 @@ describe('NftController', () => { const callActionSpy = jest .spyOn(messenger, 'call') .mockReturnValueOnce(OWNER_ACCOUNT) - .mockResolvedValueOnce({}) - .mockReturnValue(OWNER_ACCOUNT); + .mockResolvedValue({}); await nftController.watchNft( ERC1155_NFT, ERC1155, 'https://etherscan.io', ); - // First call is getInternalAccount. Second call is the approval request. - expect(callActionSpy).toHaveBeenCalledTimes(3); + // First call is `AccountsController:getAccount` + // Fifth call is `ApprovalController:addRequest` + // Sixth call is `AccountsController:getAccount` + expect(callActionSpy).toHaveBeenCalledTimes(6); expect(callActionSpy).toHaveBeenNthCalledWith( - 2, + 5, 'ApprovalController:addRequest', { id: requestId, From 1559242831dd2cc299ea5fd5c2dd1babfbc728a5 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 15 Jul 2024 21:44:48 -0400 Subject: [PATCH 25/40] Set `ApprovalController:addRequest` payload `requestData.asset.standard` property fallback value to `suggestedNftMeta.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 949041f651..292f470c20 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -1989,7 +1989,7 @@ export class NftController extends BaseController< name: suggestedNftMeta.asset.name, description: suggestedNftMeta.asset.description, image: suggestedNftMeta.asset.image, - standard: suggestedNftMeta.asset.standard, + standard: suggestedNftMeta.asset.standard ?? suggestedNftMeta.type, }, }, }, From ed70562abfb0daeaed72a11e6e1074b965c79d6e Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 15 Jul 2024 21:45:44 -0400 Subject: [PATCH 26/40] Fix async `expect.rejects.toThrow` call so that the `expect` call is synchronous and its argument is an async callback --- .../assets-controllers/src/NftController.test.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 1faa90bee9..afd147e5df 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -1280,12 +1280,14 @@ describe('NftController', () => { const requestId = 'approval-request-id-1'; (v4 as jest.Mock).mockImplementationOnce(() => requestId); - const result = nftController.watchNft( - ERC721_NFT, - ERC721, - 'https://test-dapp.com', - ); - await expect(result).rejects.toThrow( + expect( + async () => + await nftController.watchNft( + ERC721_NFT, + ERC721, + 'https://test-dapp.com', + ), + ).rejects.toThrow( "Unable to verify ownership. Possibly because the standard is not supported or the user's currently selected network does not match the chain of the asset in question.", ); }); From ffa8cb37097301df3a3bd861d5b7b9d5c7b92370 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 15 Jul 2024 21:46:00 -0400 Subject: [PATCH 27/40] Update coverage thresholds --- packages/assets-controllers/jest.config.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index c5034d8960..7caaad6c87 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 90.35, - functions: 96.74, - lines: 97.34, - statements: 97.36, + branches: 91.23, + functions: 97.51, + lines: 98.12, + statements: 98.03, }, }, From 022b3ffe1c1a667c9227b9b1c06bb3b3d620e792 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 15 Jul 2024 21:55:46 -0400 Subject: [PATCH 28/40] Add `eslint-disable` directive to prevent test from timing out --- packages/assets-controllers/src/NftController.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index afd147e5df..a3b72a629a 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -1280,6 +1280,8 @@ describe('NftController', () => { const requestId = 'approval-request-id-1'; (v4 as jest.Mock).mockImplementationOnce(() => requestId); + // Awaiting `expect` as recommended by eslint results in this test stalling and timing out. + // eslint-disable-next-line @typescript-eslint/no-floating-promises, jest/valid-expect expect( async () => await nftController.watchNft( From 18cc886e0299c9c8da99fb56d7856fc83e95ddd7 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 16 Jul 2024 01:21:38 -0400 Subject: [PATCH 29/40] Adapt `watchNft` tests to new messenger call invocations - Explicitly mock all messenger call invocation results - Fix results for `'should handle ERC721 type and add pending request to ApprovalController with the OpenSea API enabled and IPFS gateway disabled'` test based on fully mocked return values --- .../src/NftController.test.ts | 141 +++++++++++++----- 1 file changed, 102 insertions(+), 39 deletions(-) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index a3b72a629a..9c5f93d1e1 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -657,16 +657,25 @@ describe('NftController', () => { const callActionSpy = jest .spyOn(messenger, 'call') + // 1. `AccountsController:getAccount` .mockReturnValueOnce(OWNER_ACCOUNT) - .mockResolvedValue({}); + // 2. `AssetsContractController:getERC721OwnerOf` + .mockResolvedValueOnce(OWNER_ADDRESS) + // 3. `AssetsContractController:getERC721TokenURI` + .mockResolvedValueOnce('https://testtokenuri.com') + // 4. `ApprovalController:addRequest` + .mockResolvedValueOnce({}) + // 5. `AccountsController:getAccount` + .mockReturnValueOnce(OWNER_ACCOUNT) + // 6. `AssetsContractController:getERC721AssetName` + .mockResolvedValueOnce('testERC721Name') + // 7. `AssetsContractController:getERC721AssetSymbol` + .mockResolvedValueOnce('testERC721Symbol'); await nftController.watchNft(ERC721_NFT, ERC721, 'https://test-dapp.com'); - // First call is `AccountsController:getAccount` - // Fifth call is `ApprovalController:addRequest` - // Sixth call is `AccountsController:getAccount` - expect(callActionSpy).toHaveBeenCalledTimes(6); + expect(callActionSpy).toHaveBeenCalledTimes(7); expect(callActionSpy).toHaveBeenNthCalledWith( - 5, + 4, 'ApprovalController:addRequest', { id: requestId, @@ -727,16 +736,25 @@ describe('NftController', () => { const callActionSpy = jest .spyOn(messenger, 'call') + // 1. `AccountsController:getAccount` + .mockReturnValueOnce(OWNER_ACCOUNT) + // 2. `AssetsContractController:getERC721OwnerOf` + .mockResolvedValueOnce(OWNER_ADDRESS) + // 3. `AssetsContractController:getERC721TokenURI` + .mockResolvedValueOnce('https://testtokenuri.com') + // 4. `ApprovalController:addRequest` + .mockResolvedValueOnce({}) + // 5. `AccountsController:getAccount` .mockReturnValueOnce(OWNER_ACCOUNT) - .mockResolvedValue({}); + // 6. `AssetsContractController:getERC721AssetName` + .mockResolvedValueOnce('testERC721Name') + // 7. `AssetsContractController:getERC721AssetSymbol` + .mockResolvedValueOnce('testERC721Symbol'); await nftController.watchNft(ERC721_NFT, ERC721, 'https://test-dapp.com'); - // First call is `AccountsController:getAccount` - // Fifth call is `ApprovalController:addRequest` - // Sixth call is `AccountsController:getAccount` - expect(callActionSpy).toHaveBeenCalledTimes(6); + expect(callActionSpy).toHaveBeenCalledTimes(7); expect(callActionSpy).toHaveBeenNthCalledWith( - 5, + 4, 'ApprovalController:addRequest', { id: requestId, @@ -797,16 +815,25 @@ describe('NftController', () => { const callActionSpy = jest .spyOn(messenger, 'call') + // 1. `AccountsController:getAccount` .mockReturnValueOnce(OWNER_ACCOUNT) - .mockResolvedValue({}); + // 2. `AssetsContractController:getERC721OwnerOf` + .mockResolvedValueOnce(OWNER_ADDRESS) + // 3. `AssetsContractController:getERC721TokenURI` + .mockResolvedValueOnce('https://testtokenuri.com') + // 4. `ApprovalController:addRequest` + .mockResolvedValueOnce({}) + // 5. `AccountsController:getAccount` + .mockReturnValueOnce(OWNER_ACCOUNT) + // 6. `AssetsContractController:getERC721AssetName` + .mockResolvedValueOnce('testERC721Name') + // 7. `AssetsContractController:getERC721AssetSymbol` + .mockResolvedValueOnce('testERC721Symbol'); await nftController.watchNft(ERC721_NFT, ERC721, 'https://test-dapp.com'); - // First call is `AccountsController:getAccount` - // Fifth call is `ApprovalController:addRequest` - // Sixth call is `AccountsController:getAccount` - expect(callActionSpy).toHaveBeenCalledTimes(6); + expect(callActionSpy).toHaveBeenCalledTimes(7); expect(callActionSpy).toHaveBeenNthCalledWith( - 5, + 4, 'ApprovalController:addRequest', { id: requestId, @@ -868,16 +895,25 @@ describe('NftController', () => { const callActionSpy = jest .spyOn(messenger, 'call') + // 1. `AccountsController:getAccount` + .mockReturnValueOnce(OWNER_ACCOUNT) + // 2. `AssetsContractController:getERC721OwnerOf` + .mockResolvedValueOnce(OWNER_ADDRESS) + // 3. `AssetsContractController:getERC721TokenURI` + .mockResolvedValueOnce('https://testtokenuri.com') + // 4. `ApprovalController:addRequest` + .mockResolvedValueOnce({}) + // 5. `AccountsController:getAccount` .mockReturnValueOnce(OWNER_ACCOUNT) - .mockResolvedValue({}); + // 6. `AssetsContractController:getERC721AssetName` + .mockResolvedValueOnce('testERC721Name') + // 7. `AssetsContractController:getERC721AssetSymbol` + .mockResolvedValueOnce('testERC721Symbol'); await nftController.watchNft(ERC721_NFT, ERC721, 'https://test-dapp.com'); - // First call is `AccountsController:getAccount` - // Fifth call is `ApprovalController:addRequest` - // Sixth call is `AccountsController:getAccount` - expect(callActionSpy).toHaveBeenCalledTimes(6); + expect(callActionSpy).toHaveBeenCalledTimes(7); expect(callActionSpy).toHaveBeenNthCalledWith( - 5, + 4, 'ApprovalController:addRequest', { id: requestId, @@ -888,9 +924,9 @@ describe('NftController', () => { interactingAddress: OWNER_ADDRESS, asset: { ...ERC721_NFT, - description: null, - image: null, - name: null, + description: 'testERC721Description', + image: 'testERC721Image', + name: 'testERC721Name', standard: ERC721, }, }, @@ -942,20 +978,33 @@ describe('NftController', () => { const callActionSpy = jest .spyOn(messenger, 'call') + // 1. `AccountsController:getAccount` .mockReturnValueOnce(OWNER_ACCOUNT) - .mockResolvedValue({}); + // 2. `AssetsContractController:getERC721OwnerOf` + .mockRejectedValueOnce(new Error('Not an ERC721 contract')) + // 3. `AssetsContractController:getERC1155BalanceOf` + .mockResolvedValueOnce(new BN(1)) + // 4. `AssetsContractController:getERC721TokenURI` + .mockRejectedValueOnce(new Error('Not an ERC721 contract')) + // 5. `AssetsContractController:getERC1155TokenURI` + .mockResolvedValueOnce('https://testtokenuri.com') + // 6. `ApprovalController:addRequest` + .mockResolvedValueOnce({}) + // 7. `AccountsController:getAccount` + .mockReturnValueOnce(OWNER_ACCOUNT) + // 8. `AssetsContractController:getERC1155AssetName` + .mockResolvedValueOnce('testERC1155Name') + // 9. `AssetsContractController:getERC1155AssetSymbol` + .mockResolvedValueOnce('testERC1155Symbol'); await nftController.watchNft( ERC1155_NFT, ERC1155, 'https://etherscan.io', ); - // First call is `AccountsController:getAccount` - // Fifth call is `ApprovalController:addRequest` - // Sixth call is `AccountsController:getAccount` - expect(callActionSpy).toHaveBeenCalledTimes(6); + expect(callActionSpy).toHaveBeenCalledTimes(9); expect(callActionSpy).toHaveBeenNthCalledWith( - 5, + 6, 'ApprovalController:addRequest', { id: requestId, @@ -1014,20 +1063,34 @@ describe('NftController', () => { const callActionSpy = jest .spyOn(messenger, 'call') + // 1. `AccountsController:getAccount` + .mockReturnValueOnce(OWNER_ACCOUNT) + // 2. `AssetsContractController:getERC721OwnerOf` + .mockRejectedValueOnce(new Error('Not an ERC721 contract')) + // 3. `AssetsContractController:getERC1155BalanceOf` + .mockResolvedValueOnce(new BN(1)) + // 4. `AssetsContractController:getERC721TokenURI` + .mockRejectedValueOnce(new Error('Not an ERC721 contract')) + // 5. `AssetsContractController:getERC1155TokenURI` + .mockResolvedValueOnce('https://testtokenuri.com') + // 6. `ApprovalController:addRequest` + .mockResolvedValueOnce({}) + // 7. `AccountsController:getAccount` .mockReturnValueOnce(OWNER_ACCOUNT) - .mockResolvedValue({}); + // 8. `AssetsContractController:getERC1155AssetName` + .mockResolvedValueOnce('testERC1155Name') + // 9. `AssetsContractController:getERC1155AssetSymbol` + .mockResolvedValueOnce('testERC1155Symbol'); await nftController.watchNft( ERC1155_NFT, ERC1155, 'https://etherscan.io', ); - // First call is `AccountsController:getAccount` - // Fifth call is `ApprovalController:addRequest` - // Sixth call is `AccountsController:getAccount` - expect(callActionSpy).toHaveBeenCalledTimes(6); + + expect(callActionSpy).toHaveBeenCalledTimes(9); expect(callActionSpy).toHaveBeenNthCalledWith( - 5, + 6, 'ApprovalController:addRequest', { id: requestId, From c44538473419132126025c2fcd2e1a8ea473271e Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 16 Jul 2024 01:45:27 -0400 Subject: [PATCH 30/40] Use `getKnownPropertyNames` for iterative `registerActionHandlers` logic --- .../src/AssetsContractController.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 7b32ffa93e..52e22cc595 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -15,7 +15,7 @@ import type { Provider, } from '@metamask/network-controller'; import type { PreferencesControllerStateChangeEvent } from '@metamask/preferences-controller'; -import type { Hex } from '@metamask/utils'; +import { getKnownPropertyNames, type Hex } from '@metamask/utils'; import type BN from 'bn.js'; import abiSingleCallBalancesContract from 'single-call-balance-checker-abi'; @@ -213,9 +213,7 @@ export class AssetsContractController { 'chainId', ]; - for (const method of Object.getOwnPropertyNames( - Object.getPrototypeOf(this), - ) as (keyof this)[]) { + getKnownPropertyNames(this).forEach((method) => { if ( ((key: keyof this): key is AssetsContractControllerMethodName => !nonMethodClassProperties.find((e) => e === key) && @@ -223,12 +221,12 @@ export class AssetsContractController { ) { this.messagingSystem.registerActionHandler( `${name}:${method}`, - // TODO: Write a for-loop function that iterates over an input union type in tandem with the input array. - // @ts-expect-error Both assigned argument and assignee parameter are using the entire union type for `method` + // TODO: Write a generic for-loop implementation that iterates over an input union type in tandem with the input array. + // @ts-expect-error Both assigned argument and assignee parameter are using the entire union type for `method` instead of the type for the current element this[method].bind(this), ); } - } + }); } #registerEventSubscriptions() { From 056e8dc4b5ed82da7b0a4293e74cab65429f8664 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 16 Jul 2024 02:26:10 -0400 Subject: [PATCH 31/40] Revert unnecessary changes --- packages/assets-controllers/jest.config.js | 2 +- packages/assets-controllers/src/NftController.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 7caaad6c87..a226e79eb7 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -17,7 +17,7 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 91.23, + branches: 91.07, functions: 97.51, lines: 98.12, statements: 98.03, diff --git a/packages/assets-controllers/src/NftController.ts b/packages/assets-controllers/src/NftController.ts index 292f470c20..7ea8328567 100644 --- a/packages/assets-controllers/src/NftController.ts +++ b/packages/assets-controllers/src/NftController.ts @@ -1420,7 +1420,7 @@ export class NftController extends BaseController< tokenId, networkClientId, ); - return String(balance) !== '0'; + return !balance.isZero(); // eslint-disable-next-line no-empty } catch { // Ignore ERC-1155 contract error @@ -1989,7 +1989,7 @@ export class NftController extends BaseController< name: suggestedNftMeta.asset.name, description: suggestedNftMeta.asset.description, image: suggestedNftMeta.asset.image, - standard: suggestedNftMeta.asset.standard ?? suggestedNftMeta.type, + standard: suggestedNftMeta.asset.standard, }, }, }, From 99fc5b99d196fdf44d2e4d5eb9b0914c42fc4ccb Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 16 Jul 2024 02:27:13 -0400 Subject: [PATCH 32/40] Remove unreleased changelog entries --- packages/assets-controllers/CHANGELOG.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 33f8b2c0e0..869c8c5b06 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -7,10 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Added - -- Add and export new types `AssetsContractControllerMessenger`, `AssetsContractControllerActions`, `AssetsContractControllerEvents`, `AssetsContractControllerGetERC20StandardAction`, `AssetsContractControllerGetERC721StandardAction`, `AssetsContractControllerGetERC1155StandardAction`, `AssetsContractControllerGetERC20BalanceOfAction`, `AssetsContractControllerGetERC20TokenDecimalsAction`, `AssetsContractControllerGetERC20TokenNameAction`, `AssetsContractControllerGetERC721NftTokenIdAction`, `AssetsContractControllerGetERC721TokenURIAction`, `AssetsContractControllerGetERC721AssetNameAction`, `AssetsContractControllerGetERC721AssetSymbolAction`, `AssetsContractControllerGetERC721OwnerOfAction`, `AssetsContractControllerGetERC1155TokenURIAction`, `AssetsContractControllerGetERC1155BalanceOfAction`, `AssetsContractControllerTransferSingleERC1155Action`, `AssetsContractControllerGetTokenStandardAndDetailsAction`, `AssetsContractControllerGetBalancesInSingleCallAction` [#4397](https://github.com/MetaMask/core/pull/4397) - ## [35.0.0] ### Changed From 6c6f6e51b3fb7e048eadb257215d64d63dd692c1 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 16 Jul 2024 12:11:20 -0400 Subject: [PATCH 33/40] Convert `provider` setter into `setProvider` method --- .../src/AssetsContractController.test.ts | 46 +++++++++---------- .../src/AssetsContractController.ts | 6 +-- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.test.ts b/packages/assets-controllers/src/AssetsContractController.test.ts index e7c1d81daa..d2dce7fd15 100644 --- a/packages/assets-controllers/src/AssetsContractController.test.ts +++ b/packages/assets-controllers/src/AssetsContractController.test.ts @@ -242,7 +242,7 @@ describe('AssetsContractController', () => { it('should throw missing provider error when getting ERC-20 token balance when missing provider', async () => { const { assetsContract, messenger } = await setupAssetContractControllers(); - assetsContract.provider = undefined; + assetsContract.setProvider(undefined); await expect( messenger.call( `AssetsContractController:getERC20BalanceOf`, @@ -255,7 +255,7 @@ describe('AssetsContractController', () => { it('should throw missing provider error when getting ERC-20 token decimal when missing provider', async () => { const { assetsContract, messenger } = await setupAssetContractControllers(); - assetsContract.provider = undefined; + assetsContract.setProvider(undefined); await expect( messenger.call( `AssetsContractController:getERC20TokenDecimals`, @@ -268,7 +268,7 @@ describe('AssetsContractController', () => { it('should get balance of ERC-20 token contract correctly', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -324,7 +324,7 @@ describe('AssetsContractController', () => { it('should get ERC-721 NFT tokenId correctly', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -358,7 +358,7 @@ describe('AssetsContractController', () => { it('should throw missing provider error when getting ERC-721 token standard and details when missing provider', async () => { const { assetsContract, messenger } = await setupAssetContractControllers(); - assetsContract.provider = undefined; + assetsContract.setProvider(undefined); await expect( messenger.call( `AssetsContractController:getTokenStandardAndDetails`, @@ -372,7 +372,7 @@ describe('AssetsContractController', () => { it('should throw contract standard error when getting ERC-20 token standard and details when provided with invalid ERC-20 address', async () => { const { assetsContract, messenger, provider } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); const error = 'Unable to determine contract standard'; await expect( messenger.call( @@ -387,7 +387,7 @@ describe('AssetsContractController', () => { it('should get ERC-721 token standard and details', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -453,7 +453,7 @@ describe('AssetsContractController', () => { it('should get ERC-1155 token standard and details', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -539,7 +539,7 @@ describe('AssetsContractController', () => { it('should get ERC-20 token standard and details', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -637,7 +637,7 @@ describe('AssetsContractController', () => { it('should get ERC-721 NFT tokenURI correctly', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -687,7 +687,7 @@ describe('AssetsContractController', () => { it('should not throw an error when address given does not support NFT Metadata interface', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); const errorLogSpy = jest .spyOn(console, 'error') .mockImplementationOnce(() => { @@ -746,7 +746,7 @@ describe('AssetsContractController', () => { it('should get ERC-721 NFT name', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -779,7 +779,7 @@ describe('AssetsContractController', () => { it('should get ERC-721 NFT symbol', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -823,7 +823,7 @@ describe('AssetsContractController', () => { it('should get ERC-20 token decimals', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -856,7 +856,7 @@ describe('AssetsContractController', () => { it('should get ERC-20 token name', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -891,7 +891,7 @@ describe('AssetsContractController', () => { it('should get ERC-721 NFT ownership', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -937,7 +937,7 @@ describe('AssetsContractController', () => { it('should get balance of ERC-20 token in a single call on network with token detection support', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -1044,7 +1044,7 @@ describe('AssetsContractController', () => { useNetworkControllerProvider: true, infuraProjectId, }); - assetsContract.provider = provider; + assetsContract.setProvider(provider); const balancesOnMainnet = await messenger.call( 'AssetsContractController:getBalancesInSingleCall', @@ -1079,7 +1079,7 @@ describe('AssetsContractController', () => { provider, networkClientConfiguration, } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -1152,7 +1152,7 @@ describe('AssetsContractController', () => { it('should throw missing provider error when transferring single ERC-1155 when missing provider', async () => { const { assetsContract, messenger } = await setupAssetContractControllers(); - assetsContract.provider = undefined; + assetsContract.setProvider(undefined); await expect( messenger.call( `AssetsContractController:transferSingleERC1155`, @@ -1169,7 +1169,7 @@ describe('AssetsContractController', () => { it('should throw when ERC1155 function transferSingle is not defined', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -1207,7 +1207,7 @@ describe('AssetsContractController', () => { it('should get the balance of a ERC-1155 NFT for a given address', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -1255,7 +1255,7 @@ describe('AssetsContractController', () => { it('should get the URI of a ERC-1155 NFT', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); - assetsContract.provider = provider; + assetsContract.setProvider(provider); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 52e22cc595..82e9df44d1 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -254,11 +254,9 @@ export class AssetsContractController { /** * Sets a new provider. * - * TODO: Replace this wth a method. - * - * @property provider - Provider used to create a new underlying Web3 instance + * @param provider - Provider used to create a new underlying Web3 instance */ - set provider(provider: Provider | undefined) { + setProvider(provider: Provider | undefined) { this.#provider = provider; } From 20e5451f3527c6e98b7f2ff73efa7a49b27a8351 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 16 Jul 2024 13:10:10 -0400 Subject: [PATCH 34/40] Add jsdoc for `AssetsContractController` types --- .../src/AssetsContractController.ts | 41 ++++++++++++++++--- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 82e9df44d1..27d1be9215 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -82,21 +82,38 @@ export type BalanceMap = { [tokenAddress: string]: BN; }; +/** + * The name of the {@link AssetsContractController} + */ const name = 'AssetsContractController'; -// TODO: Convert into generic type that takes controller type as input and move to base-controller -type AssetsContractControllerActionsMap = { - [ClassMethod in keyof AssetsContractController as AssetsContractController[ClassMethod] extends ActionConstraint['handler'] +/** + * A utility type that derives the public method names of a given messenger consumer class, + * and uses it to generate the class's internal messenger action types. + * @template Controller - A messenger consumer class. + */ +// TODO: Figure out generic constraint and move to base-controller +type ControllerActionsMap = { + [ClassMethod in keyof Controller as Controller[ClassMethod] extends ActionConstraint['handler'] ? ClassMethod : never]: { - type: `${typeof name}:${ClassMethod}`; - handler: AssetsContractController[ClassMethod]; + type: `${typeof name}:${ClassMethod & string}`; + handler: Controller[ClassMethod]; }; }; +type AssetsContractControllerActionsMap = + ControllerActionsMap; + +/** + * The union of all public class method names of {@link AssetsContractController}. + */ type AssetsContractControllerMethodName = keyof AssetsContractControllerActionsMap; +/** + * The union of all internal messenger actions available to the {@link AssetsContractControllerMessenger}. + */ export type AssetsContractControllerActions = AssetsContractControllerActionsMap[AssetsContractControllerMethodName]; @@ -148,18 +165,30 @@ export type AssetsContractControllerGetTokenStandardAndDetailsAction = export type AssetsContractControllerGetBalancesInSingleCallAction = AssetsContractControllerActionsMap['getBalancesInSingleCall']; +/** + * The union of all internal messenger events available to the {@link AssetsContractControllerMessenger}. + */ export type AssetsContractControllerEvents = never; +/** + * The union of all external messenger actions that must be allowed by the {@link AssetsContractControllerMessenger}. + */ export type AllowedActions = | NetworkControllerGetNetworkClientByIdAction | NetworkControllerGetNetworkConfigurationByNetworkClientId | NetworkControllerGetSelectedNetworkClientAction | NetworkControllerGetStateAction; +/** + * The union of all external messenger event that must be allowed by the {@link AssetsContractControllerMessenger}. + */ export type AllowedEvents = | PreferencesControllerStateChangeEvent | NetworkControllerNetworkDidChangeEvent; +/** + * The messenger of the {@link AssetsContractController}. + */ export type AssetsContractControllerMessenger = RestrictedControllerMessenger< typeof name, AssetsContractControllerActions | AllowedActions, @@ -184,7 +213,7 @@ export class AssetsContractController { * Creates a AssetsContractController instance. * * @param options - The controller options. - * @param options.messenger - + * @param options.messenger - The controller messenger. * @param options.chainId - The chain ID of the current network. */ constructor({ From 1d5ea6662c9eddd4d3d8555e798d5b50f8200845 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 16 Jul 2024 13:46:58 -0400 Subject: [PATCH 35/40] Fix `#registerActionHandlers` - Restore usage of `Object.getPrototypeOf` - add `setProvider` to exceptions --- .../src/AssetsContractController.ts | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 27d1be9215..8a6825de20 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -237,25 +237,28 @@ export class AssetsContractController { const nonMethodClassProperties = [ 'constructor', 'messagingSystem', + 'setProvider', 'provider', 'ipfsGateway', 'chainId', ]; - getKnownPropertyNames(this).forEach((method) => { - if ( - ((key: keyof this): key is AssetsContractControllerMethodName => - !nonMethodClassProperties.find((e) => e === key) && - typeof this[key] === 'function')(method) - ) { - this.messagingSystem.registerActionHandler( - `${name}:${method}`, - // TODO: Write a generic for-loop implementation that iterates over an input union type in tandem with the input array. - // @ts-expect-error Both assigned argument and assignee parameter are using the entire union type for `method` instead of the type for the current element - this[method].bind(this), - ); - } - }); + getKnownPropertyNames(Object.getPrototypeOf(this)).forEach( + (method) => { + if ( + ((key: keyof this): key is AssetsContractControllerMethodName => + !nonMethodClassProperties.find((e) => e === key) && + typeof this[key] === 'function')(method) + ) { + this.messagingSystem.registerActionHandler( + `${name}:${method}`, + // TODO: Write a generic for-loop implementation that iterates over an input union type in tandem with the input array. + // @ts-expect-error Both assigned argument and assignee parameter are using the entire union type for `method` instead of the type for the current element + this[method].bind(this), + ); + } + }, + ); } #registerEventSubscriptions() { From 236844a6b21f303fc9e8e9dc75997aef1aeaef6f Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 16 Jul 2024 18:42:50 -0400 Subject: [PATCH 36/40] test(nft-controller): fix messenger call mocks --- .../assets-controllers/src/NftController.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 9c5f93d1e1..9a4cde0059 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -992,10 +992,10 @@ describe('NftController', () => { .mockResolvedValueOnce({}) // 7. `AccountsController:getAccount` .mockReturnValueOnce(OWNER_ACCOUNT) - // 8. `AssetsContractController:getERC1155AssetName` - .mockResolvedValueOnce('testERC1155Name') - // 9. `AssetsContractController:getERC1155AssetSymbol` - .mockResolvedValueOnce('testERC1155Symbol'); + // 8. `AssetsContractController:getERC721AssetName` + .mockRejectedValueOnce(new Error('Not an ERC721 contract')) + // 9. `AssetsContractController:getERC721AssetSymbol` + .mockRejectedValueOnce(new Error('Not an ERC721 contract')); await nftController.watchNft( ERC1155_NFT, @@ -1077,10 +1077,10 @@ describe('NftController', () => { .mockResolvedValueOnce({}) // 7. `AccountsController:getAccount` .mockReturnValueOnce(OWNER_ACCOUNT) - // 8. `AssetsContractController:getERC1155AssetName` - .mockResolvedValueOnce('testERC1155Name') - // 9. `AssetsContractController:getERC1155AssetSymbol` - .mockResolvedValueOnce('testERC1155Symbol'); + // 8. `AssetsContractController:getERC721AssetName` + .mockRejectedValueOnce(new Error('Not an ERC721 contract')) + // 9. `AssetsContractController:getERC721AssetSymbol` + .mockRejectedValueOnce(new Error('Not an ERC721 contract')); await nftController.watchNft( ERC1155_NFT, From a40f0d1b5996822d270f16f659f2dbcc4883d323 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 16 Jul 2024 22:49:56 -0400 Subject: [PATCH 37/40] test(nft-controller): expose mock constructors for allowedActions --- .../src/NftController.test.ts | 100 +++++++++++------- 1 file changed, 62 insertions(+), 38 deletions(-) diff --git a/packages/assets-controllers/src/NftController.test.ts b/packages/assets-controllers/src/NftController.test.ts index 9a4cde0059..ed79b7037d 100644 --- a/packages/assets-controllers/src/NftController.test.ts +++ b/packages/assets-controllers/src/NftController.test.ts @@ -1,5 +1,9 @@ import type { Network } from '@ethersproject/providers'; -import type { AccountsControllerSelectedAccountChangeEvent } from '@metamask/accounts-controller'; +import type { + AccountsControllerGetAccountAction, + AccountsControllerGetSelectedAccountAction, + AccountsControllerSelectedAccountChangeEvent, +} from '@metamask/accounts-controller'; import type { ApprovalControllerMessenger } from '@metamask/approval-controller'; import { ApprovalController } from '@metamask/approval-controller'; import { ControllerMessenger } from '@metamask/base-controller'; @@ -146,6 +150,10 @@ jest.mock('uuid', () => { * @param args.mockNetworkClientConfigurationsByNetworkClientId - Used to construct * mock versions of network clients and ultimately mock the * `NetworkController:getNetworkClientById` action. + * @param args.getAccount - Used to construct mock versions of the + * `AccountsController:getAccount` action. + * @param args.getSelectedAccount - Used to construct mock versions of the + * `AccountsController:getSelectedAccount` action. * @param args.defaultSelectedAccount - The default selected account to use in * @returns A collection of test controllers and mocks. */ @@ -157,6 +165,8 @@ function setupController({ getERC721OwnerOf, getERC1155BalanceOf, getERC1155TokenURI, + getAccount, + getSelectedAccount, mockNetworkClientConfigurationsByNetworkClientId = {}, defaultSelectedAccount = OWNER_ACCOUNT, }: { @@ -185,6 +195,14 @@ function setupController({ ReturnType, Parameters >; + getAccount?: jest.Mock< + ReturnType, + Parameters | [null] + >; + getSelectedAccount?: jest.Mock< + ReturnType, + Parameters + >; mockNetworkClientConfigurationsByNetworkClientId?: Record< NetworkClientId, NetworkClientConfiguration @@ -209,19 +227,15 @@ function setupController({ getNetworkClientById, ); - const mockGetAccount = jest - .fn() - .mockReturnValue(defaultSelectedAccount ?? OWNER_ACCOUNT); - + const mockGetAccount = + getAccount ?? jest.fn().mockReturnValue(defaultSelectedAccount); messenger.registerActionHandler( 'AccountsController:getAccount', mockGetAccount, ); - const mockGetSelectedAccount = jest - .fn() - .mockReturnValue(defaultSelectedAccount ?? OWNER_ACCOUNT); - + const mockGetSelectedAccount = + getSelectedAccount ?? jest.fn().mockReturnValue(defaultSelectedAccount); messenger.registerActionHandler( 'AccountsController:getSelectedAccount', mockGetSelectedAccount, @@ -637,10 +651,13 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ + getAccount: jest.fn().mockReturnValue(OWNER_ACCOUNT), + getERC721OwnerOf: jest.fn().mockResolvedValue(OWNER_ADDRESS), getERC721TokenURI: jest .fn() - .mockImplementation(() => 'https://testtokenuri.com'), - getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), + .mockResolvedValue('https://testtokenuri.com'), + getERC721AssetName: jest.fn().mockResolvedValue('testERC721Name'), + getERC721AssetSymbol: jest.fn().mockResolvedValue('testERC721Symbol'), }); triggerSelectedAccountChange(OWNER_ACCOUNT); triggerPreferencesStateChange({ @@ -716,10 +733,13 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ + getAccount: jest.fn().mockReturnValue(OWNER_ACCOUNT), + getERC721OwnerOf: jest.fn().mockResolvedValue(OWNER_ADDRESS), getERC721TokenURI: jest .fn() - .mockImplementation(() => 'https://testtokenuri.com'), - getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), + .mockResolvedValue('https://testtokenuri.com'), + getERC721AssetName: jest.fn().mockResolvedValue('testERC721Name'), + getERC721AssetSymbol: jest.fn().mockResolvedValue('testERC721Symbol'), }); triggerSelectedAccountChange(OWNER_ACCOUNT); triggerPreferencesStateChange({ @@ -795,10 +815,13 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ + getAccount: jest.fn().mockReturnValue(OWNER_ACCOUNT), + getERC721OwnerOf: jest.fn().mockResolvedValue(OWNER_ADDRESS), getERC721TokenURI: jest .fn() - .mockImplementation(() => 'ipfs://testtokenuri.com'), - getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), + .mockResolvedValue('https://testtokenuri.com'), + getERC721AssetName: jest.fn().mockResolvedValue('testERC721Name'), + getERC721AssetSymbol: jest.fn().mockResolvedValue('testERC721Symbol'), }); triggerSelectedAccountChange(OWNER_ACCOUNT); triggerPreferencesStateChange({ @@ -874,10 +897,13 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ + getAccount: jest.fn().mockReturnValue(OWNER_ACCOUNT), + getERC721OwnerOf: jest.fn().mockResolvedValue(OWNER_ADDRESS), getERC721TokenURI: jest .fn() - .mockImplementation(() => 'ipfs://testtokenuri.com'), - getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), + .mockResolvedValue('https://testtokenuri.com'), + getERC721AssetName: jest.fn().mockResolvedValue('testERC721Name'), + getERC721AssetSymbol: jest.fn().mockResolvedValue('testERC721Symbol'), }); triggerSelectedAccountChange(OWNER_ACCOUNT); @@ -955,13 +981,17 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ + getAccount: jest.fn().mockReturnValue(OWNER_ACCOUNT), + getERC721OwnerOf: jest + .fn() + .mockRejectedValue(new Error('Not an ERC721 contract')), + getERC1155BalanceOf: jest.fn().mockResolvedValue(new BN(1)), getERC721TokenURI: jest .fn() .mockRejectedValue(new Error('Not an ERC721 contract')), getERC1155TokenURI: jest .fn() - .mockImplementation(() => 'https://testtokenuri.com'), - getERC1155BalanceOf: jest.fn().mockImplementation(() => new BN(1)), + .mockResolvedValue('https://testtokenuri.com'), }); triggerSelectedAccountChange(OWNER_ACCOUNT); @@ -1042,13 +1072,17 @@ describe('NftController', () => { const { nftController, messenger, triggerPreferencesStateChange } = setupController({ + getAccount: jest.fn().mockReturnValue(OWNER_ACCOUNT), + getERC721OwnerOf: jest + .fn() + .mockRejectedValue(new Error('Not an ERC721 contract')), + getERC1155BalanceOf: jest.fn().mockResolvedValue(new BN(1)), getERC721TokenURI: jest .fn() .mockRejectedValue(new Error('Not an ERC721 contract')), getERC1155TokenURI: jest .fn() - .mockImplementation(() => 'https://testtokenuri.com'), - getERC1155BalanceOf: jest.fn().mockImplementation(() => new BN(1)), + .mockResolvedValue('https://testtokenuri.com'), }); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), @@ -1134,18 +1168,12 @@ describe('NftController', () => { triggerPreferencesStateChange, triggerSelectedAccountChange, } = setupController({ - getERC721OwnerOf: jest - .fn() - .mockImplementation(() => SECOND_OWNER_ADDRESS), + getERC721OwnerOf: jest.fn().mockResolvedValue(SECOND_OWNER_ADDRESS), getERC721TokenURI: jest .fn() - .mockImplementation(() => 'https://testtokenuri.com'), - getERC721AssetName: jest - .fn() - .mockImplementation(() => 'testERC721Name'), - getERC721AssetSymbol: jest - .fn() - .mockImplementation(() => 'testERC721Symbol'), + .mockResolvedValue('https://testtokenuri.com'), + getERC721AssetName: jest.fn().mockResolvedValue('testERC721Name'), + getERC721AssetSymbol: jest.fn().mockResolvedValue('testERC721Symbol'), }); const requestId = 'approval-request-id-1'; @@ -1241,13 +1269,9 @@ describe('NftController', () => { getERC721OwnerOf: jest.fn().mockImplementation(() => OWNER_ADDRESS), getERC721TokenURI: jest .fn() - .mockImplementation(() => 'https://testtokenuri.com'), - getERC721AssetName: jest - .fn() - .mockImplementation(() => 'testERC721Name'), - getERC721AssetSymbol: jest - .fn() - .mockImplementation(() => 'testERC721Symbol'), + .mockResolvedValue('https://testtokenuri.com'), + getERC721AssetName: jest.fn().mockResolvedValue('testERC721Name'), + getERC721AssetSymbol: jest.fn().mockResolvedValue('testERC721Symbol'), }); const requestId = 'approval-request-id-1'; From f50e5f4c8be72338f83eaf845ff4492b12660196 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 17 Jul 2024 12:48:00 -0400 Subject: [PATCH 38/40] Rename variable for clarity Co-authored-by: Elliot Winkler --- packages/assets-controllers/src/AssetsContractController.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index 8a6825de20..fb0d557406 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -234,7 +234,7 @@ export class AssetsContractController { // TODO: Expand into base-controller utility function that batch registers action handlers. #registerActionHandlers() { - const nonMethodClassProperties = [ + const methodsExcludedFromMessenger = [ 'constructor', 'messagingSystem', 'setProvider', @@ -247,7 +247,7 @@ export class AssetsContractController { (method) => { if ( ((key: keyof this): key is AssetsContractControllerMethodName => - !nonMethodClassProperties.find((e) => e === key) && + !methodsExcludedFromMessenger.find((e) => e === key) && typeof this[key] === 'function')(method) ) { this.messagingSystem.registerActionHandler( From 418df796688bf6da7cf8946489e90f30318d648f Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 17 Jul 2024 12:58:28 -0400 Subject: [PATCH 39/40] Remove getter for `provider` --- .../src/AssetsContractController.test.ts | 8 -------- .../assets-controllers/src/AssetsContractController.ts | 4 ---- 2 files changed, 12 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.test.ts b/packages/assets-controllers/src/AssetsContractController.test.ts index d2dce7fd15..a1a943e50e 100644 --- a/packages/assets-controllers/src/AssetsContractController.test.ts +++ b/packages/assets-controllers/src/AssetsContractController.test.ts @@ -232,14 +232,6 @@ describe('AssetsContractController', () => { messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); - it('should throw when provider property is accessed', async () => { - const { assetsContract, messenger } = await setupAssetContractControllers(); - expect(() => console.log(assetsContract.provider)).toThrow( - 'Property only used for setting', - ); - messenger.clearEventSubscriptions('NetworkController:networkDidChange'); - }); - it('should throw missing provider error when getting ERC-20 token balance when missing provider', async () => { const { assetsContract, messenger } = await setupAssetContractControllers(); assetsContract.setProvider(undefined); diff --git a/packages/assets-controllers/src/AssetsContractController.ts b/packages/assets-controllers/src/AssetsContractController.ts index fb0d557406..d3fd83b4d6 100644 --- a/packages/assets-controllers/src/AssetsContractController.ts +++ b/packages/assets-controllers/src/AssetsContractController.ts @@ -292,10 +292,6 @@ export class AssetsContractController { this.#provider = provider; } - get provider() { - throw new Error('Property only used for setting'); - } - get ipfsGateway() { return this.#ipfsGateway; } From 49831d9d77a0b3daf5aa99c39043391df3ae5622 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 17 Jul 2024 16:53:54 -0400 Subject: [PATCH 40/40] Apply `ExtractAvailable{Action,Event}` test utility types Co-authored-by: Elliot Winkler --- .../src/AssetsContractController.test.ts | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.test.ts b/packages/assets-controllers/src/AssetsContractController.test.ts index a1a943e50e..277405bbec 100644 --- a/packages/assets-controllers/src/AssetsContractController.test.ts +++ b/packages/assets-controllers/src/AssetsContractController.test.ts @@ -23,13 +23,12 @@ import { getDefaultPreferencesState } from '@metamask/preferences-controller'; import assert from 'assert'; import { mockNetwork } from '../../../tests/mock-network'; -import { buildInfuraNetworkClientConfiguration } from '../../network-controller/tests/helpers'; import type { - AssetsContractControllerActions, - AssetsContractControllerEvents, - AllowedActions as AssetsContractAllowedActions, - AllowedEvents as AssetsContractAllowedEvents, -} from './AssetsContractController'; + ExtractAvailableAction, + ExtractAvailableEvent, +} from '../../base-controller/tests/helpers'; +import { buildInfuraNetworkClientConfiguration } from '../../network-controller/tests/helpers'; +import type { AssetsContractControllerMessenger } from './AssetsContractController'; import { AssetsContractController, MISSING_PROVIDER_ERROR, @@ -80,12 +79,10 @@ async function setupAssetContractControllers({ let provider: Provider; const controllerMessenger = new ControllerMessenger< - | NetworkControllerActions - | AssetsContractControllerActions - | AssetsContractAllowedActions, + | ExtractAvailableAction + | NetworkControllerActions, + | ExtractAvailableEvent | NetworkControllerEvents - | AssetsContractControllerEvents - | AssetsContractAllowedEvents >(); const networkController = new NetworkController({ infuraProjectId,