Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: migrate NftDetectionController to BaseControllerV2 #4312

Merged
merged 21 commits into from
May 30, 2024
Merged

feat: migrate NftDetectionController to BaseControllerV2 #4312

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
ea7539b
feat: migrate NftDetectionController to BaseControllerV2
cryptodev-2s May 23, 2024
0d6e4fa
fix: shadowed state variable
cryptodev-2s May 23, 2024
73dcdaf
fix: imports order
cryptodev-2s May 23, 2024
2dbae95
Merge branch 'main' into feature/migrate-nft-detection-controller-to-…
cryptodev-2s May 27, 2024
243e284
Merge branch 'main' into feature/migrate-nft-detection-controller-to-…
cryptodev-2s May 27, 2024
8a29197
fix: use messenger
cryptodev-2s May 28, 2024
26777c7
fix: add missing approval request action
cryptodev-2s May 28, 2024
7979390
Merge branch 'main' into feature/migrate-nft-detection-controller-to-…
cryptodev-2s May 28, 2024
95e7bbd
fix: extract nft api timeout and version to vars
cryptodev-2s May 28, 2024
3b2b882
fix: unit test typos
cryptodev-2s May 28, 2024
2ca4649
fix: remove chaind id from state
cryptodev-2s May 28, 2024
c0d3ab7
fix: simplify network state mock
cryptodev-2s May 28, 2024
3837642
fix: remove state
cryptodev-2s May 28, 2024
a10248e
fix: state generic type
cryptodev-2s May 28, 2024
b7811f0
fix: handle case nft response is nullable
cryptodev-2s May 28, 2024
fbb2198
fix: unit tests
cryptodev-2s May 28, 2024
175db9e
fix: state type use empty object
cryptodev-2s May 28, 2024
92d8364
Merge branch 'main' into feature/migrate-nft-detection-controller-to-…
cryptodev-2s May 28, 2024
5f1c2be
Merge branch 'main' into feature/migrate-nft-detection-controller-to-…
cryptodev-2s May 29, 2024
d3df568
fix: increase test cov
cryptodev-2s May 29, 2024
69ace07
Merge branch 'main' into feature/migrate-nft-detection-controller-to-…
cryptodev-2s May 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
173 changes: 101 additions & 72 deletions packages/assets-controllers/src/NftDetectionController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { NFT_API_BASE_URL, ChainId, toHex } from '@metamask/controller-utils';
import { ControllerMessenger } from '@metamask/base-controller';
import { NFT_API_BASE_URL, ChainId } from '@metamask/controller-utils';
import {
NetworkClientType,
defaultState as defaultNetworkState,
Expand All @@ -24,15 +25,18 @@ import {
buildMockGetNetworkClientById,
} from '../../network-controller/tests/helpers';
import { Source } from './constants';
import { getDefaultNftState, type NftState } from './NftController';
import { getDefaultNftState } from './NftController';
import {
type NftDetectionConfig,
NftDetectionController,
BlockaidResultType,
type AllowedActions,
type AllowedEvents,
} from './NftDetectionController';

const DEFAULT_INTERVAL = 180000;

const controllerName = 'NftDetectionController' as const;

describe('NftDetectionController', () => {
let clock: sinon.SinonFakeTimers;

Expand Down Expand Up @@ -283,25 +287,23 @@ describe('NftDetectionController', () => {
sinon.restore();
});

it('should set default config', async () => {
it('should set default state', async () => {
await withController(({ controller }) => {
expect(controller.config).toStrictEqual({
interval: DEFAULT_INTERVAL,
chainId: toHex(1),
expect(controller.state).toStrictEqual({
chainId: '0x1',
selectedAddress: '',
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
disabled: true,
});
});
});

it('should poll and detect NFTs on interval while on mainnet', async () => {
await withController(
{ config: { interval: 10 } },
{ options: { interval: 10 } },
async ({ controller, controllerEvents }) => {
const mockNfts = sinon
.stub(controller, 'detectNfts')
.returns(Promise.resolve());
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
useNftDetection: true,
});
Expand Down Expand Up @@ -425,7 +427,7 @@ describe('NftDetectionController', () => {
],
]);

controllerEvents.networkStateChange({
controllerEvents.triggerNetworkStateChange({
...defaultNetworkState,
selectedNetworkClientId: 'AAAA-AAAA-AAAA-AAAA',
});
Expand Down Expand Up @@ -460,13 +462,36 @@ describe('NftDetectionController', () => {
);
});

it('should detect mainnet correctly', async () => {
await withController(({ controller }) => {
controller.configure({ chainId: ChainId.mainnet });
expect(controller.isMainnet()).toBe(true);
controller.configure({ chainId: ChainId.goerli });
expect(controller.isMainnet()).toBe(false);
});
it('should detect mainnet truty', async () => {
await withController(
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
{
options: {
state: {
chainId: ChainId.mainnet,
selectedAddress: '',
},
},
},
({ controller }) => {
expect(controller.isMainnet()).toBe(true);
},
);
});

it('should detect mainnet falsy', async () => {
await withController(
{
options: {
state: {
chainId: ChainId.goerli,
selectedAddress: '',
},
},
},
({ controller }) => {
expect(controller.isMainnet()).toBe(false);
},
);
});

it('should not autodetect while not on mainnet', async () => {
Expand All @@ -486,14 +511,14 @@ describe('NftDetectionController', () => {

await withController(
{
config: {
interval: pollingInterval,
},
options: {
interval: pollingInterval,
addNft: mockAddNft,
chainId: '0x1',
disabled: false,
selectedAddress: '0x1',
state: {
chainId: '0x1',
selectedAddress: '0x1',
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
},
},
mockNetworkClientConfigurationsByNetworkClientId: {
'AAAA-AAAA-AAAA-AAAA': buildCustomNetworkClientConfiguration({
Expand Down Expand Up @@ -561,7 +586,7 @@ describe('NftDetectionController', () => {
},
);

controllerEvents.networkStateChange({
controllerEvents.triggerNetworkStateChange({
...defaultNetworkState,
selectedNetworkClientId: 'AAAA-AAAA-AAAA-AAAA',
});
Expand All @@ -580,7 +605,7 @@ describe('NftDetectionController', () => {
{ options: { addNft: mockAddNft } },
async ({ controller, controllerEvents }) => {
const selectedAddress = '0x1';
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
selectedAddress,
useNftDetection: true,
Expand Down Expand Up @@ -620,7 +645,7 @@ describe('NftDetectionController', () => {
{ options: { addNft: mockAddNft } },
async ({ controller, controllerEvents }) => {
const selectedAddress = '0x123';
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
selectedAddress,
useNftDetection: true,
Expand Down Expand Up @@ -669,7 +694,7 @@ describe('NftDetectionController', () => {
{ options: { addNft: mockAddNft } },
async ({ controller, controllerEvents }) => {
const selectedAddress = '0x12345';
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
selectedAddress,
useNftDetection: true,
Expand Down Expand Up @@ -730,7 +755,7 @@ describe('NftDetectionController', () => {
{ options: { addNft: mockAddNft } },
async ({ controller, controllerEvents }) => {
const selectedAddress = '0x1';
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
selectedAddress,
useNftDetection: true,
Expand Down Expand Up @@ -787,7 +812,7 @@ describe('NftDetectionController', () => {
{ options: { addNft: mockAddNft, getNftState: mockGetNftState } },
async ({ controller, controllerEvents }) => {
const selectedAddress = '0x9';
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
selectedAddress,
useNftDetection: true,
Expand All @@ -812,14 +837,14 @@ describe('NftDetectionController', () => {
{ options: { addNft: mockAddNft } },
async ({ controller, controllerEvents }) => {
const selectedAddress = ''; // Emtpy selected address
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
selectedAddress,
useNftDetection: true, // auto-detect is enabled so it proceeds to check userAddress
});

// confirm that default selected address is an empty string
expect(controller.config.selectedAddress).toBe('');
expect(controller.state.selectedAddress).toBe('');

await controller.detectNfts();

Expand All @@ -832,7 +857,7 @@ describe('NftDetectionController', () => {
const mockAddNft = jest.fn();
const mockNetworkClient: NetworkClient = {
configuration: {
chainId: toHex(1),
chainId: '0x1',
rpcUrl: 'https://test.network',
cryptodev-2s marked this conversation as resolved.
Show resolved Hide resolved
ticker: 'TEST',
type: NetworkClientType.Custom,
Expand All @@ -854,10 +879,10 @@ describe('NftDetectionController', () => {

it('should not detectNfts when disabled is false and useNftDetection is true', async () => {
await withController(
{ config: { interval: 10 }, options: { disabled: false } },
{ options: { disabled: false, interval: 10 } },
async ({ controller, controllerEvents }) => {
const mockNfts = sinon.stub(controller, 'detectNfts');
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
useNftDetection: true,
});
Expand Down Expand Up @@ -885,7 +910,7 @@ describe('NftDetectionController', () => {
{ options: { addNft: mockAddNft } },
async ({ controller, controllerEvents }) => {
const selectedAddress = '0x9';
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
selectedAddress,
useNftDetection: false,
Expand Down Expand Up @@ -920,7 +945,7 @@ describe('NftDetectionController', () => {
await withController(
{ options: { addNft: mockAddNft } },
async ({ controller, controllerEvents }) => {
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
selectedAddress,
useNftDetection: true,
Expand Down Expand Up @@ -954,7 +979,7 @@ describe('NftDetectionController', () => {
.reply(200, {
tokens: [],
});
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
selectedAddress,
useNftDetection: true,
Expand Down Expand Up @@ -987,7 +1012,7 @@ describe('NftDetectionController', () => {
{ options: { addNft: mockAddNft } },
async ({ controller, controllerEvents }) => {
const selectedAddress = '0x1';
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
selectedAddress,
useNftDetection: true,
Expand All @@ -1013,7 +1038,7 @@ describe('NftDetectionController', () => {

// Repeated preference changes should only trigger 1 detection
for (let i = 0; i < 5; i++) {
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
useNftDetection: true,
});
Expand All @@ -1022,7 +1047,7 @@ describe('NftDetectionController', () => {
expect(detectNfts.callCount).toBe(1);

// Irrelevant preference changes shouldn't trigger a detection
controllerEvents.preferencesStateChange({
controllerEvents.triggerPreferencesStateChange({
...getDefaultPreferencesState(),
useNftDetection: true,
securityAlertsEnabled: true,
Expand All @@ -1037,9 +1062,8 @@ describe('NftDetectionController', () => {
* A collection of mock external controller events.
*/
type ControllerEvents = {
nftsStateChange: (state: NftState) => void;
preferencesStateChange: (state: PreferencesState) => void;
networkStateChange: (state: NetworkState) => void;
triggerPreferencesStateChange: (state: PreferencesState) => void;
triggerNetworkStateChange: (state: NetworkState) => void;
};

type WithControllerCallback<ReturnValue> = ({
Expand All @@ -1051,7 +1075,6 @@ type WithControllerCallback<ReturnValue> = ({

type WithControllerOptions = {
options?: Partial<ConstructorParameters<typeof NftDetectionController>[0]>;
config?: Partial<NftDetectionConfig>;
mockNetworkClientConfigurationsByNetworkClientId?: Record<
NetworkClientId,
NetworkClientConfiguration
Expand All @@ -1075,45 +1098,51 @@ async function withController<ReturnValue>(
...args: WithControllerArgs<ReturnValue>
): Promise<ReturnValue> {
const [
{
options = {},
config = {},
mockNetworkClientConfigurationsByNetworkClientId = {},
},
{ options = {}, mockNetworkClientConfigurationsByNetworkClientId = {} },
testFunction,
] = args.length === 2 ? args : [{}, args[0]];

// Explicit cast used here because we know the `on____` functions are always
// set in the constructor.
const controllerEvents = {} as ControllerEvents;
const messenger = new ControllerMessenger<AllowedActions, AllowedEvents>();

const getNetworkClientById = buildMockGetNetworkClientById(
mockNetworkClientConfigurationsByNetworkClientId,
);
messenger.registerActionHandler(
'NetworkController:getNetworkClientById',
getNetworkClientById,
);

const controllerMessenger = messenger.getRestricted({
name: controllerName,
allowedActions: ['NetworkController:getNetworkClientById'],
allowedEvents: [
'NetworkController:stateChange',
'PreferencesController:stateChange',
],
});

const controller = new NftDetectionController(
{
const controller = new NftDetectionController({
state: {
chainId: ChainId.mainnet,
onNftsStateChange: (listener) => {
controllerEvents.nftsStateChange = listener;
},
onPreferencesStateChange: (listener) => {
controllerEvents.preferencesStateChange = listener;
},
onNetworkStateChange: (listener) => {
controllerEvents.networkStateChange = listener;
},
getOpenSeaApiKey: jest.fn(),
addNft: jest.fn(),
getNftApi: jest.fn(),
getNetworkClientById,
getNftState: getDefaultNftState,
disabled: true,
selectedAddress: '',
...options,
...(options?.state || {}),
},
config,
);
messenger: controllerMessenger,
disabled: true,
addNft: jest.fn(),
getNftState: getDefaultNftState,
...options,
});

const controllerEvents = {
triggerPreferencesStateChange: (state: PreferencesState) => {
messenger.publish('PreferencesController:stateChange', state, []);
},
triggerNetworkStateChange: (state: NetworkState) => {
messenger.publish('NetworkController:stateChange', state, []);
},
};

try {
return await testFunction({
controller,
Expand Down
Loading
Loading