Merged
Conversation
xavier-brochard
commented
Sep 3, 2025
Comment on lines
-26
to
-27
| export type GetTokenAccountsByOwnerResponse<TToken> = | ||
| readonly AccountInfoWithPubkey<AccountInfoBase & TToken>[]; |
Contributor
Author
There was a problem hiding this comment.
Cleaning up this unused type
xavier-brochard
commented
Sep 4, 2025
| import type { AssetsService, TokenHelper } from '../assets'; | ||
| import { TransactionMapper } from './TransactionMapper'; | ||
|
|
||
| describe('TransactionMapper', () => { |
Contributor
Author
There was a problem hiding this comment.
This test file simply contains all previously existing unit tests from transactions/utils:
utils/mapRpcTransaction.test.tsutils/parseTransactionSpltTransfers.test.ts
etc...
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
xavier-brochard
commented
Sep 4, 2025
| import { tokenAddressToCaip19 } from '../../utils/tokenAddressToCaip19'; | ||
| import type { AssetMetadata } from '../assets/types'; | ||
|
|
||
| export class TransactionMapper { |
Contributor
Author
There was a problem hiding this comment.
This class simply gathers all methods that previously existed in transactions/utils:
utils/mapRpcTransaction.tsutils/parseTransactionSplTransfers.ts- etc...
This change was necessary because the low level method parseTransactionSplTransfers now needs to depend on TokenHelper to convert raw amount to uiAmount.
xavier-brochard
commented
Sep 4, 2025
Comment on lines
-179
to
-282
| /** | ||
| * Get the token account for a given mint and network. | ||
| * @param params - The parameters object containing mint and network. | ||
| * @param params.mint - The mint address. | ||
| * @param params.network - The network. | ||
| * @returns The token account. Handle with care, it might: | ||
| * - not exist (exists: false). | ||
| * - be encoded (data instanceof Uint8Array). | ||
| */ | ||
| async getTokenAccount<TData extends Uint8Array | object>({ | ||
| mint, | ||
| network, | ||
| }: { | ||
| mint: Address; | ||
| network: Network; | ||
| }): Promise<MaybeAccount<TData> | MaybeEncodedAccount> { | ||
| const rpc = this.#connection.getRpc(network); | ||
| const tokenAccount = await fetchJsonParsedAccount<TData>(rpc, mint); | ||
|
|
||
| return tokenAccount; | ||
| } | ||
|
|
||
| /** | ||
| * Get the decimals of a given token account. | ||
| * @param tokenAccount - The token account. | ||
| * @returns The decimals. | ||
| */ | ||
| getDecimals<TData extends Uint8Array | MaybeHasDecimals>( | ||
| tokenAccount: MaybeAccount<TData> | MaybeEncodedAccount, | ||
| ): number { | ||
| SendSplTokenBuilder.assertAccountExists(tokenAccount); | ||
| SendSplTokenBuilder.assertAccountDecoded(tokenAccount); | ||
|
|
||
| const { decimals } = tokenAccount.data; | ||
|
|
||
| if (!decimals) { | ||
| throw new Error(`Decimals not found for ${tokenAccount}`); | ||
| } | ||
|
|
||
| return decimals; | ||
| } | ||
|
|
||
| /** | ||
| * Check if a token account exists. | ||
| * @param tokenAccount - The token account. | ||
| * @returns Whether the token account exists. | ||
| */ | ||
| static isAccountExists<TData extends Uint8Array | object>( | ||
| tokenAccount: MaybeAccount<TData> | MaybeEncodedAccount, | ||
| ): boolean { | ||
| return tokenAccount.exists; | ||
| } | ||
|
|
||
| /** | ||
| * Assert that a token account exists. | ||
| * @param tokenAccount - The token account. | ||
| */ | ||
| static assertAccountExists<TData extends Uint8Array | object>( | ||
| tokenAccount: MaybeAccount<TData> | MaybeEncodedAccount, | ||
| ): asserts tokenAccount is (MaybeAccount<TData> | MaybeEncodedAccount) & | ||
| Exists { | ||
| if (!SendSplTokenBuilder.isAccountExists(tokenAccount)) { | ||
| throw new Error('Token account does not exist'); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Assert that a token account does not exists. | ||
| * @param tokenAccount - The token account. | ||
| */ | ||
| static assertAccountNotExists<TData extends Uint8Array | object>( | ||
| tokenAccount: MaybeAccount<TData> | MaybeEncodedAccount, | ||
| ): asserts tokenAccount is (MaybeAccount<TData> | MaybeEncodedAccount) & | ||
| Exists { | ||
| if (SendSplTokenBuilder.isAccountExists(tokenAccount)) { | ||
| throw new Error('Token account exists'); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Check if a token account is decoded. | ||
| * @param tokenAccount - The token account. | ||
| * @returns Whether the token account is decoded. | ||
| */ | ||
| static isAccountDecoded<TData extends Uint8Array | object>( | ||
| tokenAccount: MaybeAccount<TData> | MaybeEncodedAccount, | ||
| ): boolean { | ||
| SendSplTokenBuilder.assertAccountExists(tokenAccount); | ||
| return !(tokenAccount.data instanceof Uint8Array); | ||
| } | ||
|
|
||
| /** | ||
| * Assert that a token account is decoded. | ||
| * @param tokenAccount - The token account. | ||
| */ | ||
| static assertAccountDecoded<TData extends Uint8Array | object>( | ||
| tokenAccount: MaybeAccount<TData> | MaybeEncodedAccount, | ||
| ): asserts tokenAccount is Account<Exclude<TData, Uint8Array>> & Exists { | ||
| SendSplTokenBuilder.assertAccountExists(tokenAccount); | ||
| if (!SendSplTokenBuilder.isAccountDecoded(tokenAccount)) { | ||
| throw new Error('Token account is encoded. Implement a decoder.'); | ||
| } | ||
| } | ||
|
|
Contributor
Author
There was a problem hiding this comment.
All these were unused, and the Solana SDK export equivalent methods in case we need.
xavier-brochard
commented
Sep 4, 2025
Comment on lines
-231
to
-356
|
|
||
| describe('getTokenAccount', () => { | ||
| const mockMint = '4zMMC9srt5Ri5X14GAgXhaHii3GnPAEERYPJgZJDncDU' as Address; | ||
| const mockNetwork = Network.Localnet; | ||
|
|
||
| it('returns token account when it exists', async () => { | ||
| const mockTokenAccount = { | ||
| exists: true, | ||
| address: mockMint, | ||
| data: { decimals: 6 }, | ||
| } as unknown as MaybeAccount<any>; | ||
|
|
||
| jest.spyOn(mockConnection, 'getRpc').mockReturnValue({ | ||
| // Mock the minimum required RPC methods | ||
| getAccountInfo: jest.fn(), | ||
| } as unknown as Rpc<SolanaRpcApi>); | ||
|
|
||
| // Mock the web3.js fetchJsonParsedAccount function | ||
| const fetchJsonParsedAccountSpy = jest.spyOn( | ||
| require('@solana/kit'), | ||
| 'fetchJsonParsedAccount', | ||
| ); | ||
| fetchJsonParsedAccountSpy.mockResolvedValue(mockTokenAccount); | ||
|
|
||
| const result = await sendSplTokenBuilder.getTokenAccount({ | ||
| mint: mockMint, | ||
| network: mockNetwork, | ||
| }); | ||
|
|
||
| expect(result).toStrictEqual(mockTokenAccount); | ||
| expect(mockConnection.getRpc).toHaveBeenCalledWith(mockNetwork); | ||
| expect(fetchJsonParsedAccountSpy).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('returns non-existing account when token account does not exist', async () => { | ||
| const mockNonExistingAccount = { | ||
| exists: false, | ||
| } as unknown as MaybeAccount<any>; | ||
|
|
||
| jest.spyOn(mockConnection, 'getRpc').mockReturnValue({ | ||
| getAccountInfo: jest.fn(), | ||
| } as unknown as Rpc<SolanaRpcApi>); | ||
|
|
||
| const fetchJsonParsedAccountSpy = jest.spyOn( | ||
| require('@solana/kit'), | ||
| 'fetchJsonParsedAccount', | ||
| ); | ||
| fetchJsonParsedAccountSpy.mockResolvedValue(mockNonExistingAccount); | ||
|
|
||
| const result = await sendSplTokenBuilder.getTokenAccount({ | ||
| mint: mockMint, | ||
| network: mockNetwork, | ||
| }); | ||
|
|
||
| expect(result).toStrictEqual(mockNonExistingAccount); | ||
| expect(mockConnection.getRpc).toHaveBeenCalledWith(mockNetwork); | ||
| expect(fetchJsonParsedAccountSpy).toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('getDecimals', () => { | ||
| it('returns decimals from token account', () => { | ||
| const mockTokenAccount = { | ||
| exists: true, | ||
| data: { decimals: 6 }, | ||
| } as unknown as MaybeAccount<MaybeHasDecimals> & Exists; | ||
|
|
||
| const result = sendSplTokenBuilder.getDecimals(mockTokenAccount); | ||
| expect(result).toBe(6); | ||
| }); | ||
|
|
||
| it('throws error if account does not exist', () => { | ||
| const mockTokenAccount = { | ||
| exists: false, | ||
| } as unknown as MaybeAccount<any>; | ||
|
|
||
| expect(() => sendSplTokenBuilder.getDecimals(mockTokenAccount)).toThrow( | ||
| 'Token account does not exist', | ||
| ); | ||
| }); | ||
|
|
||
| it('throws error if decimals are not found', () => { | ||
| const mockTokenAccount = { | ||
| exists: true, | ||
| data: {}, | ||
| } as unknown as MaybeAccount<any> & Exists; | ||
|
|
||
| expect(() => sendSplTokenBuilder.getDecimals(mockTokenAccount)).toThrow( | ||
| 'Decimals not found', | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('assertAccountDecoded', () => { | ||
| it('does not throw for decoded account', () => { | ||
| const mockDecodedAccount = { | ||
| exists: true, | ||
| data: { someField: 'value' }, | ||
| } as unknown as MaybeAccount<any> & Exists; | ||
|
|
||
| expect(() => { | ||
| SendSplTokenBuilder.assertAccountDecoded(mockDecodedAccount); | ||
| }).not.toThrow(); | ||
| }); | ||
|
|
||
| it('throws for encoded account (Uint8Array)', () => { | ||
| const mockEncodedAccount = { | ||
| exists: true, | ||
| data: new Uint8Array([1, 2, 3]), | ||
| } as unknown as MaybeAccount<any> & Exists; | ||
|
|
||
| expect(() => { | ||
| SendSplTokenBuilder.assertAccountDecoded(mockEncodedAccount); | ||
| }).toThrow('Token account is encoded. Implement a decoder.'); | ||
| }); | ||
|
|
||
| it('throws for non-existent account', () => { | ||
| const mockNonExistentAccount = { | ||
| exists: false, | ||
| data: { someField: 'value' }, | ||
| } as unknown as MaybeAccount<any>; | ||
|
|
||
| expect(() => { | ||
| SendSplTokenBuilder.assertAccountDecoded(mockNonExistentAccount); | ||
| }).toThrow('Token account does not exist'); | ||
| }); |
Contributor
Author
There was a problem hiding this comment.
Related methods were removed because unused
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
xavier-brochard
commented
Sep 4, 2025
Comment on lines
+382
to
+394
| /** | ||
| * WARNING: This is to compensate for the fact that the notification returned by Infura's programSubscribe | ||
| * includes a uiAmount/uiAmountString that does not take into account the mint's multiplier (if any). | ||
| * In theory, it should; because the regular Solana RPC (wss://api.mainnet-beta.solana.com) does. | ||
| * | ||
| * So this needs to be removed once Infura fixes their programSubscribe notification. | ||
| */ | ||
| const uiAmount = await this.#tokenHelper | ||
| .amountToUiAmountForMint(mint, network, lamports(BigInt(amount))) | ||
| .catch((error) => { | ||
| this.#logger.error('Error converting amount to uiAmount', error); | ||
| return uiAmountString; | ||
| }); |
Contributor
Author
There was a problem hiding this comment.
What should we do about this?
- Support on our side and not request anything from Infura?
- Support temporarily until Infura supports?
Contributor
There was a problem hiding this comment.
how long is it going to take for them to support it?
Contributor
Author
There was a problem hiding this comment.
I don't know yet. I've explained the situation to Gabriel, and he is analyzing the need now.
@aganglada
xavier-brochard
commented
Sep 4, 2025
Comment on lines
-285
to
-330
| async #mapRpcTransactionToKeyringTransaction( | ||
| transactionData: SolanaTransaction | null, | ||
| account: SolanaKeyringAccount, | ||
| scope: Network, | ||
| assetsMetadata?: Record<string, AssetMetadata | null>, | ||
| ): Promise<Transaction | null> { | ||
| if (!transactionData) { | ||
| return null; | ||
| } | ||
|
|
||
| const mappedTransaction = mapRpcTransaction({ | ||
| transactionData, | ||
| account, | ||
| scope, | ||
| }); | ||
|
|
||
| if (!mappedTransaction) { | ||
| return null; | ||
| } | ||
|
|
||
| const caip19Ids = [ | ||
| ...new Set( | ||
| [...mappedTransaction.from, ...mappedTransaction.to] | ||
| .filter((item) => item.asset?.fungible) | ||
| .map((item) => (item.asset as { type: CaipAssetType }).type), | ||
| ), | ||
| ]; | ||
|
|
||
| const assetsMetadataToUse = | ||
| assetsMetadata ?? | ||
| (await this.#assetsService.getAssetsMetadata(caip19Ids)); | ||
|
|
||
| mappedTransaction.from.forEach((from) => { | ||
| if (from.asset?.fungible && assetsMetadataToUse[from.asset.type]) { | ||
| from.asset.unit = assetsMetadataToUse[from.asset.type]?.symbol ?? ''; | ||
| } | ||
| }); | ||
|
|
||
| mappedTransaction.to.forEach((to) => { | ||
| if (to.asset?.fungible && assetsMetadataToUse[to.asset.type]) { | ||
| to.asset.unit = assetsMetadataToUse[to.asset.type]?.symbol ?? ''; | ||
| } | ||
| }); | ||
|
|
||
| return mappedTransaction; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This logic is getting embedded in the TransactionMapper now, so that it encapsulates the whole mapping logic.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
zone-live
approved these changes
Sep 5, 2025
This was referenced Sep 5, 2025
github-merge-queue bot
pushed a commit
to MetaMask/metamask-extension
that referenced
this pull request
Sep 5, 2025
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR bumps the Solana Wallet Snap to its latest version `2.3.7`. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Added support to Solana tokens with multiplier ([#509](MetaMask/snap-solana-wallet#509)) CHANGELOG entry: Fix a bug that was causing to show spam Solana transactions in the activity list ([#515](MetaMask/snap-solana-wallet#515)) CHANGELOG entry: Fixed an issue that was causing to show an empty symbol instead of `UNKNOWN` in activity list for Solana tokens with no metadata ([#517](MetaMask/snap-solana-wallet#517)) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
github-merge-queue bot
pushed a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Sep 5, 2025
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR bumps the Solana Wallet Snap to its latest version `2.3.7`. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Added support to Solana tokens with multiplier ([#509](MetaMask/snap-solana-wallet#509)) CHANGELOG entry: Fix a bug that was causing to show spam Solana transactions in the activity list ([#515](MetaMask/snap-solana-wallet#515)) CHANGELOG entry: Fixed an issue that was causing to show an empty symbol instead of `UNKNOWN` in activity list for Solana tokens with no metadata ([#517](MetaMask/snap-solana-wallet#517)) ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces support to tokens with multipliers:
programSubscribenotificationsBecause converting from raw amount to uiAmount requires to fetch the mint account, we also need to perform a lot of refactors in the codebase:
TokenHelperservice that encapsulates the conversion logicTransactionMapperservice that depends onTokenHelper