Skip to content
1 change: 1 addition & 0 deletions packages/solana-wallet-snap/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- `keyring_getAccountBalances` now fails when an RPC fetch for a requested network fails, instead of returning a partial or empty balance, so the wallet shows the balance as unavailable rather than stale ([#527](https://github.com/MetaMask/internal-snaps/pull/527))
- Preserve accounts loaded from address lookup tables when decoding transaction instructions, preventing incomplete account lists and parsing errors during transaction history synchronization. ([#568](https://github.com/MetaMask/internal-snaps/pull/568))

## [7.3.0]
Expand Down
2 changes: 1 addition & 1 deletion packages/solana-wallet-snap/snap.manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://github.com/MetaMask/internal-snaps.git"
},
"source": {
"shasum": "Dr868Npsp/5wyPjq3W8nLkJF4MDZupuJO8yV8z8aJz4=",
"shasum": "5juA4eLDLJwq36TEKUVSaLNRG5dMuSCMTPLir8kJ1js=",
"location": {
"npm": {
"filePath": "dist/bundle.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import type {
KeyringAccountMonitor,
TransactionsService,
} from '../../services';
import { AccountAssetsFetchError } from '../../services/assets/adapters/SnapAssetsAdapter';
import type { ConfirmationHandler } from '../../services/confirmation/ConfirmationHandler';
import { DEFAULT_UNENCRYPTED_STATE } from '../../services/state/stateTypes';
import type { UnencryptedStateValue } from '../../services/state/stateTypes';
Expand Down Expand Up @@ -396,6 +397,7 @@ describe('SolanaKeyring', () => {

expect(mockAssetsService.fetchAccountAssetsFromRpc).toHaveBeenCalledWith(
MOCK_SOLANA_KEYRING_ACCOUNT_0,
{ failOnNetworks: [MOCK_ASSET_ENTITY_1.assetType.split('/')[0]] },
);
expect(result).toStrictEqual({
[MOCK_ASSET_ENTITY_1.assetType]: {
Expand All @@ -418,6 +420,30 @@ describe('SolanaKeyring', () => {
).rejects.toThrow('network error');
});

it('fails instead of answering with a partial result when an RPC fetch for a requested network failed (v3)', async () => {
vi.spyOn(
mockAssetsService,
'fetchAccountAssetsFromRpc',
).mockRejectedValue(
new AccountAssetsFetchError([
{
network: Network.Mainnet,
reason: new TypeError('Failed to fetch'),
},
]),
);

await expect(
keyring.getAccountBalances(MOCK_SOLANA_KEYRING_ACCOUNT_1.id, [
KnownCaip19Id.SolMainnet,
]),
).rejects.toThrow('Failed to fetch account assets (1 failed)');
expect(mockAssetsService.fetchAccountAssetsFromRpc).toHaveBeenCalledWith(
expect.anything(),
{ failOnNetworks: [KnownCaip19Id.SolMainnet.split('/')[0]] },
);
});

it('removes token assets with zero balance', async () => {
vi.spyOn(
mockAssetsService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ import {
} from '../../utils/deriveSolanaKeypair';
import { trackError } from '../../utils/errors';
import { getBip32Entropy } from '../../utils/getBip32Entropy';
import { getNetworkFromToken } from '../../utils/getNetworkFromToken';
import {
endTrace,
listEntropySources,
Expand Down Expand Up @@ -457,8 +458,16 @@ export class SolanaKeyring implements KeyringSnapRpc {

const account = await this.getAccountOrThrow(accountId);

const assetsList =
await this.#assetsService.fetchAccountAssetsFromRpc(account);
// A failed RPC fetch for a requested network must fail the request:
// answering with the partial (or empty) result would make the client
// keep showing a stale balance with no error.
const failOnNetworks = [
...new Set(assets.map((asset) => getNetworkFromToken(asset))),
];
const assetsList = await this.#assetsService.fetchAccountAssetsFromRpc(
account,
{ failOnNetworks },
);

const assetsToUse = assetsList
.filter((asset) => assets.includes(asset.assetType))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,24 @@ describe('AssetsService', () => {
MOCK_SOLANA_KEYRING_ACCOUNT_0,
);

expect(fetchSpy).toHaveBeenCalledWith(MOCK_SOLANA_KEYRING_ACCOUNT_0);
expect(fetchSpy).toHaveBeenCalledWith(MOCK_SOLANA_KEYRING_ACCOUNT_0, {});
expect(assets).toStrictEqual(MOCK_ASSET_ENTITIES);
});

it('forwards failOnNetworks to the Snap adapter', async () => {
const fetchSpy = vi
.spyOn(snapAssetsAdapter, 'fetchAccountAssets')
.mockResolvedValue(MOCK_ASSET_ENTITIES);

await assetsService.fetchAccountAssetsFromRpc(
MOCK_SOLANA_KEYRING_ACCOUNT_0,
{ failOnNetworks: [Network.Mainnet] },
);

expect(fetchSpy).toHaveBeenCalledWith(MOCK_SOLANA_KEYRING_ACCOUNT_0, {
failOnNetworks: [Network.Mainnet],
});
});
});

describe('getAssetsMetadata', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
import type { CaipAssetType, CaipChainId } from '@metamask/utils';

import type { AssetEntity } from '../../../entities';
import type { Network } from '../../constants/solana';
import type { CoreAssetsAdapter } from './adapters/CoreAssetsAdapter';
import { SnapAssetsAdapter } from './adapters/SnapAssetsAdapter';
import type { AssetMetadata } from './types';
Expand Down Expand Up @@ -92,12 +93,16 @@ export class AssetsService {
* asset fetching: it never consults the assets migration feature flag.
*
* @param account - The account to fetch live assets for.
* @param options - Options.
* @param options.failOnNetworks - Networks whose fetch failures must throw
* instead of being skipped (see the adapter's `fetchAccountAssets`).
* @returns The live assets.
*/
async fetchAccountAssetsFromRpc(
account: ExtendedKeyringAccount,
options: { failOnNetworks?: Network[] } = {},
): Promise<AssetEntity[]> {
return this.#snapAdapter.fetchAccountAssets(account);
return this.#snapAdapter.fetchAccountAssets(account, options);
}

async save(asset: AssetEntity): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ import type { AccountsService } from '../../accounts/AccountsService';
import type { ConfigProvider } from '../../config';
import type { SolanaConnection } from '../../connection';
import type { AssetsRepository } from '../AssetsRepository';
import { SnapAssetsAdapter } from './SnapAssetsAdapter';
import {
AccountAssetsFetchError,
SnapAssetsAdapter,
} from './SnapAssetsAdapter';

describe('SnapAssetsAdapter', () => {
let snapAssetsAdapter: SnapAssetsAdapter;
Expand Down Expand Up @@ -258,4 +261,84 @@ describe('SnapAssetsAdapter', () => {
);
});
});

describe('fetchAccountAssets with failOnNetworks (v3: SOL must not fail silently)', () => {
beforeEach(() => {
(mockConfigProvider.getActiveNetworks as Mock).mockResolvedValue([
Network.Mainnet,
]);
});

const mockRpc = ({
balanceFails = false,
tokensFail = false,
}: {
balanceFails?: boolean;
tokensFail?: boolean;
}): void => {
(mockConnection.getRpc as Mock).mockReturnValue({
getBalance: vi.fn().mockReturnValue({
send: balanceFails
? vi.fn().mockRejectedValue(new TypeError('Failed to fetch'))
: vi
.fn()
.mockResolvedValue(MOCK_SOLANA_RPC_GET_BALANCE_AS_SDK_RESPONSE),
}),
getTokenAccountsByOwner: vi.fn().mockReturnValue({
send: tokensFail
? vi.fn().mockRejectedValue(new TypeError('Failed to fetch'))
: vi
.fn()
.mockResolvedValue(
MOCK_SOLANA_RPC_GET_TOKEN_ACCOUNTS_BY_OWNER_AS_SDK_RESPONSE,
),
}),
});
};

it('throws AccountAssetsFetchError when the native balance fetch of a required network fails', async () => {
mockRpc({ balanceFails: true });

const promise = snapAssetsAdapter.fetchAccountAssets(
MOCK_SOLANA_KEYRING_ACCOUNT_0,
{ failOnNetworks: [Network.Mainnet] },
);

await expect(promise).rejects.toBeInstanceOf(AccountAssetsFetchError);
await expect(promise).rejects.toThrow(
`Failed to fetch account assets (1 failed): ${Network.Mainnet}: Failed to fetch`,
);
});

it('throws when token accounts of a required network cannot be fetched', async () => {
mockRpc({ tokensFail: true });

await expect(
snapAssetsAdapter.fetchAccountAssets(MOCK_SOLANA_KEYRING_ACCOUNT_0, {
failOnNetworks: [Network.Mainnet],
}),
).rejects.toThrow('Failed to fetch account assets (2 failed)');
});

it('ignores failures on networks that are not required', async () => {
mockRpc({ balanceFails: true });

const assets = await snapAssetsAdapter.fetchAccountAssets(
MOCK_SOLANA_KEYRING_ACCOUNT_0,
{ failOnNetworks: [Network.Devnet] },
);

expect(Array.isArray(assets)).toBe(true);
});

it('keeps the lenient behaviour without failOnNetworks (sync path)', async () => {
mockRpc({ balanceFails: true, tokensFail: true });

const assets = await snapAssetsAdapter.fetchAccountAssets(
MOCK_SOLANA_KEYRING_ACCOUNT_0,
);

expect(assets).toStrictEqual([]);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,38 @@ type TokenAccountWithMetadata = {
keyringAccount: ExtendedKeyringAccount;
} & Serializable;

/**
* A failed RPC fetch while collecting an account's assets.
*/
export type AccountAssetsFetchFailure = {
network: Network;
reason: unknown;
};

/**
* Thrown when an account's assets cannot be fetched completely for the
* networks a caller needs (see `fetchAccountAssets`'s `failOnNetworks`).
*/
export class AccountAssetsFetchError extends Error {
readonly failures: AccountAssetsFetchFailure[];

constructor(failures: AccountAssetsFetchFailure[]) {
const reasons = failures
.map(({ network, reason }) => {
const message =
reason instanceof Error ? reason.message : String(reason);
return `${network}: ${message}`;
})
.join('; ');
super(
`Failed to fetch account assets (${failures.length} failed): ${reasons}`,
{ cause: failures[0]?.reason },
);
this.name = 'AccountAssetsFetchError';
this.failures = failures;
}
}

export class SnapAssetsAdapter {
readonly #logger: Logger;

Expand Down Expand Up @@ -280,12 +312,14 @@ export class SnapAssetsAdapter {
* @param accounts - The owners of the token accounts.
* @param programIds - The program ids to fetch the token accounts for.
* @param scopes - The networks to fetch the token accounts for.
* @param failures - Collects the per-network fetch failures, if given.
* @returns The token accounts augmented with the scope and the caip-19 asset type for convenience.
*/
async #fetchTokenAccountsMultiple(
accounts: ExtendedKeyringAccount[],
programIds: Address[] = [TOKEN_PROGRAM_ADDRESS, TOKEN_2022_PROGRAM_ADDRESS],
scopes: Network[] = [Network.Mainnet],
failures?: AccountAssetsFetchFailure[],
): Promise<TokenAccountWithMetadata[]> {
if (programIds.length === 0 || scopes.length === 0) {
return [];
Expand Down Expand Up @@ -332,6 +366,13 @@ export class SnapAssetsAdapter {
combinations.map(({ account }) => account.id),
);

responses.forEach((item, index) => {
const combination = combinations[index];
if (item.status === 'rejected' && combination) {
failures?.push({ network: combination.scope, reason: item.reason });
}
});

return responses.flatMap((item) =>
item.status === 'fulfilled' ? item.value : [],
);
Expand Down Expand Up @@ -383,18 +424,45 @@ export class SnapAssetsAdapter {
* @param account - The account to get the assets and balances for.
* @returns The live assets and balances of the account.
*/
/**
* Fetches the native and token assets of an account from the RPC.
*
* Individual fetch failures are reported to Sentry and left out of the
* result. Pass `failOnNetworks` when a partial result must not be used as a
* balance (e.g. `keyring_getAccountBalances`): if any fetch for one of those
* networks failed, an {@link AccountAssetsFetchError} is thrown instead, so
* the client flags the balance unavailable rather than keeping a stale one.
*
* @param account - The account.
* @param options - Options.
* @param options.failOnNetworks - Networks whose fetch failures must throw.
* @returns The account assets.
*/
async fetchAccountAssets(
account: ExtendedKeyringAccount,
options: { failOnNetworks?: Network[] } = {},
): Promise<AssetEntity[]> {
const failures: AccountAssetsFetchFailure[] = [];
const [nativeAssets, tokenAccounts] = await Promise.all([
this.#fetchNativeAssets(account),
this.#fetchNativeAssets(account, failures),
this.#fetchTokenAccountsMultiple(
[account],
[TOKEN_PROGRAM_ADDRESS, TOKEN_2022_PROGRAM_ADDRESS],
await this.#configProvider.getActiveNetworks(),
failures,
),
]);

const { failOnNetworks } = options;
if (failOnNetworks?.length) {
const blocking = failures.filter(({ network }) =>
failOnNetworks.includes(network),
);
if (blocking.length > 0) {
throw new AccountAssetsFetchError(blocking);
}
}

const assetTypes = tokenAccounts.map(
(tokenAccount) => tokenAccount.assetType,
);
Expand Down Expand Up @@ -458,6 +526,7 @@ export class SnapAssetsAdapter {

async #fetchNativeAssets(
account: ExtendedKeyringAccount,
failures?: AccountAssetsFetchFailure[],
): Promise<NativeAsset[]> {
const nativeAssetsTypes = await this.getNativeAssetTypes();

Expand Down Expand Up @@ -489,6 +558,16 @@ export class SnapAssetsAdapter {
results.map(() => account.id),
);

results.forEach((item, index) => {
const assetType = nativeAssetsTypes[index];
if (item.status === 'rejected' && assetType) {
failures?.push({
network: getNetworkFromToken(assetType),
reason: item.reason,
});
}
});

return results.flatMap((item) =>
item.status === 'fulfilled' ? item.value : [],
);
Expand Down
Loading