Skip to content
Merged
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
10 changes: 1 addition & 9 deletions eslint-suppressions.json
Original file line number Diff line number Diff line change
Expand Up @@ -270,20 +270,12 @@
"count": 3
}
},
"packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.test.ts": {
"@typescript-eslint/explicit-function-return-type": {
"count": 1
}
},
"packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts": {
"@typescript-eslint/explicit-function-return-type": {
"count": 3
"count": 2
},
"@typescript-eslint/naming-convention": {
"count": 1
},
"@typescript-eslint/no-misused-promises": {
"count": 2
}
},
"packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts": {
Expand Down
2 changes: 2 additions & 0 deletions packages/assets-controllers/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Fix stale `accountsAssets` reference in `MultichainAssetsRatesController` causing incorrect conversion rates for non-EVM chains when currency is changed ([#7424](https://github.com/MetaMask/core/pull/7424))
- `MultichainAssetsRatesController` now dynamically fetches fresh state instead of caching a reference that becomes stale due to Immer's immutable updates
- **BREAKING:** `NftDetectionController` now calls a new function `NftController:addNfts` that reduces API calls to bulk-scan to batch multiple NFTs URLs ([#7411](https://github.com/MetaMask/core/pull/7411))
- Added decimal precision (default 9dp) for `CurrencyRateController` `conversionRate` and `conversionRate` properties. ([#7324](https://github.com/MetaMask/core/pull/7324))
- This fixes any BigNumber conversion errors due to exceeding the 15 significant digit limit
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { deriveStateFromMetadata } from '@metamask/base-controller';
import type { CaipAssetType } from '@metamask/keyring-api';
import { SolScope } from '@metamask/keyring-api';
import { SolMethod } from '@metamask/keyring-api';
import { SolAccountType } from '@metamask/keyring-api';
Expand Down Expand Up @@ -146,7 +147,11 @@ const setupController = ({
ConstructorParameters<typeof MultichainAssetsRatesController>[0]
>;
accountsAssets?: InternalAccount[];
} = {}) => {
} = {}): {
controller: MultichainAssetsRatesController;
messenger: RootMessenger;
updateSpy: jest.SpyInstance;
} => {
const messenger: RootMessenger = new Messenger({
namespace: MOCK_ANY_NAMESPACE,
});
Expand Down Expand Up @@ -1106,6 +1111,172 @@ describe('MultichainAssetsRatesController', () => {
});
});

describe('dynamic asset fetching', () => {
it('should fetch rates for assets added after controller initialization', async () => {
const messenger: RootMessenger = new Messenger({
namespace: MOCK_ANY_NAMESPACE,
});

// Initially, MultichainAssetsController has no assets
let multichainAssets: Record<string, CaipAssetType[]> = {};

messenger.registerActionHandler(
'MultichainAssetsController:getState',
() => ({
accountsAssets: multichainAssets,
assetsMetadata: {
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': {
name: 'Solana',
symbol: 'SOL',
fungible: true,
iconUrl: 'https://example.com/solana.png',
units: [{ symbol: 'SOL', name: 'Solana', decimals: 9 }],
},
},
allIgnoredAssets: {},
}),
);

messenger.registerActionHandler(
'AccountsController:listMultichainAccounts',
() => [
{
id: 'account1',
address: 'sol-address-1',
options: {},
methods: [SolMethod.SignMessage, SolMethod.SignTransaction],
type: SolAccountType.DataAccount,
metadata: {
name: 'Test Solana Account',
importTime: Date.now(),
keyring: {
type: KeyringTypes.snap,
},
snap: {
name: 'Test Snap',
id: 'test-snap',
enabled: true,
},
},
scopes: [SolScope.Mainnet],
},
],
);

messenger.registerActionHandler(
'AccountsController:getSelectedMultichainAccount',
() => ({
id: 'account1',
address: 'sol-address-1',
options: {},
methods: [SolMethod.SignMessage, SolMethod.SignTransaction],
type: SolAccountType.DataAccount,
metadata: {
name: 'Test Solana Account',
importTime: Date.now(),
keyring: {
type: KeyringTypes.snap,
},
snap: {
name: 'Test Snap',
id: 'test-snap',
enabled: true,
},
},
scopes: [SolScope.Mainnet],
}),
);

messenger.registerActionHandler(
'CurrencyRateController:getState',
() => ({
currentCurrency: 'USD',
currencyRates: {},
}),
);

const snapHandler = jest.fn().mockResolvedValue({
conversionRates: {
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': {
'swift:0/iso4217:USD': {
rate: '150.00',
conversionTime: Date.now(),
},
},
},
});

messenger.registerActionHandler(
'SnapController:handleRequest',
snapHandler,
);

const multichainAssetsRatesControllerMessenger = new Messenger<
'MultichainAssetsRatesController',
AllMultichainAssetsRateControllerActions,
AllMultichainAssetsRateControllerEvents,
RootMessenger
>({
namespace: 'MultichainAssetsRatesController',
parent: messenger,
});

messenger.delegate({
messenger: multichainAssetsRatesControllerMessenger,
actions: [
'MultichainAssetsController:getState',
'AccountsController:listMultichainAccounts',
'AccountsController:getSelectedMultichainAccount',
'CurrencyRateController:getState',
'SnapController:handleRequest',
],
events: [
'KeyringController:lock',
'KeyringController:unlock',
'AccountsController:accountAdded',
'CurrencyRateController:stateChange',
'MultichainAssetsController:accountAssetListUpdated',
],
});

jest
.spyOn(KeyringClient.prototype, 'listAccountAssets')
.mockResolvedValue([]);

const controller = new MultichainAssetsRatesController({
messenger: multichainAssetsRatesControllerMessenger,
});

// Initial fetch should return empty because no assets exist yet
await controller.updateAssetsRates();
expect(controller.state.conversionRates).toStrictEqual({});
expect(snapHandler).not.toHaveBeenCalled();

// Simulate new wallet import: MultichainAssetsController now has assets
multichainAssets = {
account1: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501'],
};

jest
.spyOn(KeyringClient.prototype, 'listAccountAssets')
.mockResolvedValue([
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501',
]);

// Fetch again - should now pick up the new assets
await controller.updateAssetsRates();

// Verify that rates were fetched for the newly added asset
expect(snapHandler).toHaveBeenCalled();
expect(controller.state.conversionRates).toMatchObject({
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': {
rate: '150.00',
currency: 'swift:0/iso4217:USD',
},
});
});
});

describe('metadata', () => {
it('includes expected state in debug snapshots', () => {
const { controller } = setupController();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import type {
import type {
MultichainAssetsControllerGetStateAction,
MultichainAssetsControllerAccountAssetListUpdatedEvent,
MultichainAssetsControllerState,
} from '../MultichainAssetsController';

/**
Expand Down Expand Up @@ -203,8 +202,6 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro

#currentCurrency: CurrencyRateState['currentCurrency'];

readonly #accountsAssets: MultichainAssetsControllerState['accountsAssets'];

#isUnlocked = true;

/**
Expand Down Expand Up @@ -244,16 +241,13 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro
this.#isUnlocked = true;
});

({ accountsAssets: this.#accountsAssets } = this.messenger.call(
'MultichainAssetsController:getState',
));

({ currentCurrency: this.#currentCurrency } = this.messenger.call(
'CurrencyRateController:getState',
));

this.messenger.subscribe(
'CurrencyRateController:stateChange',
// eslint-disable-next-line @typescript-eslint/no-misused-promises
async (currentCurrency: string) => {
this.#currentCurrency = currentCurrency;
await this.updateAssetsRates();
Expand All @@ -264,6 +258,7 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro

this.messenger.subscribe(
'MultichainAssetsController:accountAssetListUpdated',
// eslint-disable-next-line @typescript-eslint/no-misused-promises
async ({ assets }) => {
const newAccountAssets = Object.entries(assets).map(
([accountId, { added }]) => ({
Expand Down Expand Up @@ -370,7 +365,7 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro
async updateAssetsRates(): Promise<void> {
const releaseLock = await this.#mutex.acquire();

return (async () => {
return (async (): Promise<void> => {
if (!this.isActive) {
return;
}
Expand Down Expand Up @@ -683,7 +678,12 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro
* @returns An array of CAIP-19 assets.
*/
#getAssetsForAccount(accountId: string): CaipAssetType[] {
return this.#accountsAssets?.[accountId] ?? [];
// Always fetch fresh state - MultichainAssetsController uses Immer which creates
// new object references on every update, so caching would become stale.
const { accountsAssets } = this.messenger.call(
'MultichainAssetsController:getState',
);
return accountsAssets?.[accountId] ?? [];
}

/**
Expand Down
Loading