Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 0 additions & 13 deletions eslint-suppressions.json
Original file line number Diff line number Diff line change
Expand Up @@ -279,19 +279,6 @@
"count": 2
}
},
"packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts": {
"@typescript-eslint/explicit-function-return-type": {
"count": 1
}
},
"packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.ts": {
"@typescript-eslint/explicit-function-return-type": {
"count": 6
},
"@typescript-eslint/no-misused-promises": {
"count": 3
}
},
"packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.test.ts": {
"no-restricted-syntax": {
"count": 3
Expand Down
8 changes: 8 additions & 0 deletions packages/assets-controllers/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- `MultichainAssetsController`: periodic Blockaid re-scan of stored SPL-style `token:` assets (default once per day) so tokens that become malicious after a prior scan are dropped; use constructor option `blockaidTokenRescanInterval` (ms), or `0` to disable. ([#8400](https://github.com/MetaMask/core/pull/8400))

### Changed

- Bump `@metamask/accounts-controller` from `^37.1.1` to `^37.2.0` ([#8363](https://github.com/MetaMask/core/pull/8363))
- Bump `@metamask/keyring-controller` from `^25.1.1` to `^25.2.0` ([#8363](https://github.com/MetaMask/core/pull/8363))
- Bump `@metamask/messenger` from `^1.0.0` to `^1.1.1` ([#8364](https://github.com/MetaMask/core/pull/8364), [#8373](https://github.com/MetaMask/core/pull/8373))

### Fixed

- `MultichainAssetsController`: fungible `token:` assets from automatic detection are no longer added when Blockaid bulk scan fails, returns empty, or omits that address (previously fail open); an explicit non-malicious per-token result from `PhishingController:bulkScanTokens` is now required before add. ([#8400](https://github.com/MetaMask/core/pull/8400))

## [103.1.1]

### Changed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,18 +242,37 @@ function getRootMessenger(): RootMessenger {
return new Messenger({ namespace: MOCK_ANY_NAMESPACE });
}

type SetupControllerResult = {
controller: MultichainAssetsController;
messenger: RootMessenger;
mockSnapHandleRequest: jest.Mock;
mockListMultichainAccounts: jest.Mock;
mockGetAllSnaps: jest.Mock;
mockGetPermissions: jest.Mock;
mockBulkScanTokens: jest.Mock;
};

/** Request shape for `PhishingController:bulkScanTokens` in tests. */
type BulkTokenScanTestRequest = {
chainId: string;
tokens: string[];
};

const setupController = ({
state = getDefaultMultichainAssetsControllerState(),
mocks,
/** `0` disables periodic Blockaid re-scan (default for tests). */
blockaidTokenRescanInterval = 0,
}: {
state?: MultichainAssetsControllerState;
blockaidTokenRescanInterval?: number;
mocks?: {
listMultichainAccounts?: InternalAccount[];
handleRequestReturnValue?: CaipAssetTypeOrId[];
getAllReturnValue?: Snap[];
getPermissionsReturnValue?: SubjectPermissions<PermissionConstraint>;
};
} = {}) => {
} = {}): SetupControllerResult => {
const messenger = getRootMessenger();

const multichainAssetsControllerMessenger: MultichainAssetsControllerMessenger =
Expand Down Expand Up @@ -310,15 +329,30 @@ const setupController = ({
),
);

const mockBulkScanTokens = jest.fn();
const mockBulkScanTokens = jest
.fn()
.mockImplementation(
(request: BulkTokenScanTestRequest): Promise<BulkTokenScanResponse> => {
const results: BulkTokenScanResponse = {};
for (const addr of request.tokens) {
results[addr] = {
result_type: TokenScanResultType.Benign,
chain: request.chainId,
address: addr,
};
}
return Promise.resolve(results);
},
);
messenger.registerActionHandler(
'PhishingController:bulkScanTokens',
mockBulkScanTokens.mockResolvedValue({}),
mockBulkScanTokens,
);

const controller = new MultichainAssetsController({
messenger: multichainAssetsControllerMessenger,
state,
blockaidTokenRescanInterval,
});

return {
Expand Down Expand Up @@ -1265,8 +1299,8 @@ describe('MultichainAssetsController', () => {
},
});

// Wait for async processing
await jestAdvanceTime({ duration: 0 });
// Wait for async processing (including Blockaid scan)
await jestAdvanceTime({ duration: 1 });

// Only the non-ignored asset should be added
expect(
Expand Down Expand Up @@ -1302,8 +1336,7 @@ describe('MultichainAssetsController', () => {
},
});

// Wait for async processing
await jestAdvanceTime({ duration: 0 });
await jestAdvanceTime({ duration: 1 });

// Ignored asset should remain filtered out and stay in ignored list
expect(
Expand Down Expand Up @@ -1333,8 +1366,7 @@ describe('MultichainAssetsController', () => {
// Simulate account being added
messenger.publish('AccountsController:accountAdded', mockSolanaAccount);

// Wait for async processing
await jestAdvanceTime({ duration: 0 });
await jestAdvanceTime({ duration: 1 });

// All assets should be added to active list (no ignored assets for new account)
expect(
Expand Down Expand Up @@ -1482,7 +1514,7 @@ describe('MultichainAssetsController', () => {
]);
});

it('keeps all tokens when bulkScanTokens throws (fail open)', async () => {
it('does not add tokens when bulkScanTokens throws (fail closed)', async () => {
const mockAccountId = 'account1';
const token = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:SomeAddr';

Expand All @@ -1504,13 +1536,10 @@ describe('MultichainAssetsController', () => {

await jestAdvanceTime({ duration: 1 });

// Token should be kept when scan throws
expect(controller.state.accountsAssets[mockAccountId]).toStrictEqual([
token,
]);
expect(controller.state.accountsAssets[mockAccountId]).toStrictEqual([]);
});

it('keeps all tokens when bulkScanTokens returns empty (API error handled internally)', async () => {
it('does not add tokens when bulkScanTokens returns empty (API error handled internally)', async () => {
const mockAccountId = 'account1';
const token = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:SomeAddr';

Expand All @@ -1533,10 +1562,7 @@ describe('MultichainAssetsController', () => {

await jestAdvanceTime({ duration: 1 });

// Token should be kept when scan returns empty (no result = fail open)
expect(controller.state.accountsAssets[mockAccountId]).toStrictEqual([
token,
]);
expect(controller.state.accountsAssets[mockAccountId]).toStrictEqual([]);
});

it('does not scan native (slip44) assets', async () => {
Expand Down Expand Up @@ -1566,7 +1592,7 @@ describe('MultichainAssetsController', () => {
expect(mockBulkScanTokens).not.toHaveBeenCalled();
});

it('keeps tokens with no result in the scan response (fail open)', async () => {
it('does not add tokens with no result in the scan response (fail closed)', async () => {
const mockAccountId = 'account1';
const knownToken =
'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:KnownAddr';
Expand Down Expand Up @@ -1598,10 +1624,8 @@ describe('MultichainAssetsController', () => {

await jestAdvanceTime({ duration: 1 });

// Both tokens should be kept (unknown token has no result, fail open)
expect(controller.state.accountsAssets[mockAccountId]).toStrictEqual([
knownToken,
unknownToken,
]);
});

Expand Down Expand Up @@ -1713,7 +1737,7 @@ describe('MultichainAssetsController', () => {

messenger.publish('AccountsController:accountAssetListUpdated', {
assets: {
[mockAccountId]: { added: tokens, removed: [] },
[mockAccountId]: { added: tokens as CaipAssetType[], removed: [] },
},
});

Expand Down Expand Up @@ -1741,7 +1765,7 @@ describe('MultichainAssetsController', () => {
).toBeUndefined();
});

it('keeps results from successful batches when one batch fails (partial fail open)', async () => {
it('drops tokens from batches that fail (partial fail closed)', async () => {
const mockAccountId = 'account1';
// 120 tokens = batch 1 (100) + batch 2 (20)
const tokens = Array.from(
Expand Down Expand Up @@ -1776,13 +1800,13 @@ describe('MultichainAssetsController', () => {
}
return Promise.resolve(results);
}
// Second batch fails
// Second batch fails — its tokens must not be added
return Promise.reject(new Error('API timeout'));
});

messenger.publish('AccountsController:accountAssetListUpdated', {
assets: {
[mockAccountId]: { added: tokens, removed: [] },
[mockAccountId]: { added: tokens as CaipAssetType[], removed: [] },
},
});

Expand All @@ -1798,14 +1822,154 @@ describe('MultichainAssetsController', () => {
),
).toBeUndefined();

// Tokens from the failed second batch (100–119) should all be kept (fail open)
for (let i = 100; i < 120; i++) {
const tokenCaip = `solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:Token${String(i).padStart(3, '0')}`;
expect(storedAssets).toContain(tokenCaip);
expect(storedAssets).not.toContain(tokenCaip);
}

// Total: 99 benign from batch 1 + 20 kept from failed batch 2 = 119
expect(storedAssets).toHaveLength(119);
expect(storedAssets).toHaveLength(99);
});

it('periodic rescan ignores SPL tokens that Blockaid later marks malicious', async () => {
const mockAccountId = 'account1';
const token =
'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:TurnsMalicious';

const { controller, mockBulkScanTokens } = setupController({
blockaidTokenRescanInterval: 60_000,
state: {
accountsAssets: { [mockAccountId]: [token] },
assetsMetadata: {},
allIgnoredAssets: {},
} as MultichainAssetsControllerState,
});

mockBulkScanTokens.mockResolvedValue({
TurnsMalicious: {
result_type: TokenScanResultType.Malicious,
chain: 'solana',
address: 'TurnsMalicious',
},
});

await jestAdvanceTime({ duration: 1 });

expect(controller.state.accountsAssets[mockAccountId]).toStrictEqual([]);
expect(controller.state.allIgnoredAssets[mockAccountId]).toStrictEqual([
token,
]);

controller.stopAllPolling();
});

it('periodic rescan leaves tokens unchanged when bulk scan batch rejects', async () => {
const mockAccountId = 'account1';
const token = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:SomeAddr';

const { controller, mockBulkScanTokens } = setupController({
blockaidTokenRescanInterval: 60_000,
state: {
accountsAssets: { [mockAccountId]: [token] },
assetsMetadata: {},
allIgnoredAssets: {},
} as MultichainAssetsControllerState,
});

mockBulkScanTokens.mockRejectedValue(new Error('network error'));

await jestAdvanceTime({ duration: 1 });

expect(controller.state.accountsAssets[mockAccountId]).toStrictEqual([
token,
]);

controller.stopAllPolling();
});

it('periodic rescan skips Blockaid when account only holds native slip44 assets', async () => {
const mockAccountId = 'account1';
const native = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501';

const { controller, mockBulkScanTokens } = setupController({
blockaidTokenRescanInterval: 60_000,
state: {
accountsAssets: { [mockAccountId]: [native] },
assetsMetadata: {},
allIgnoredAssets: {},
} as MultichainAssetsControllerState,
});

await jestAdvanceTime({ duration: 1 });

expect(mockBulkScanTokens).not.toHaveBeenCalled();
expect(controller.state.accountsAssets[mockAccountId]).toStrictEqual([
native,
]);

controller.stopAllPolling();
});

it('periodic rescan skips entries that are not CAIP asset type strings', async () => {
const mockAccountId = 'account1';
const notCaip = 'clearly-not-caip' as CaipAssetType;

const { controller, mockBulkScanTokens } = setupController({
blockaidTokenRescanInterval: 60_000,
state: {
accountsAssets: { [mockAccountId]: [notCaip] },
assetsMetadata: {},
allIgnoredAssets: {},
} as MultichainAssetsControllerState,
});

await jestAdvanceTime({ duration: 1 });

expect(mockBulkScanTokens).not.toHaveBeenCalled();

controller.stopAllPolling();
});

it('does not publish accountAssetListUpdated when periodic rescan finds no malicious tokens', async () => {
const mockAccountId = 'account1';
const token = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:StillBenign';

const { controller, mockBulkScanTokens } = setupController({
blockaidTokenRescanInterval: 60_000,
state: {
accountsAssets: { [mockAccountId]: [token] },
assetsMetadata: {},
allIgnoredAssets: {},
} as MultichainAssetsControllerState,
});

mockBulkScanTokens.mockResolvedValue({
StillBenign: {
result_type: TokenScanResultType.Benign,
chain: 'solana',
address: 'StillBenign',
},
});

const publishSpy = jest.spyOn(
(
controller as unknown as {
messenger: MultichainAssetsControllerMessenger;
}
).messenger,
'publish',
);

await jestAdvanceTime({ duration: 1 });

expect(
publishSpy.mock.calls.filter(
(call) =>
call[0] === 'MultichainAssetsController:accountAssetListUpdated',
),
).toHaveLength(0);

publishSpy.mockRestore();
controller.stopAllPolling();
});
});

Expand Down
Loading
Loading