diff --git a/packages/multichain-transactions-controller/CHANGELOG.md b/packages/multichain-transactions-controller/CHANGELOG.md index d2ed4fdcd05..d3e24486460 100644 --- a/packages/multichain-transactions-controller/CHANGELOG.md +++ b/packages/multichain-transactions-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING:** Store transactions by chain IDs ([#5756](https://github.com/MetaMask/core/pull/5756)) +- Remove Solana mainnet filtering to support other Solana networks (devnet, testnet) ([#5756](https://github.com/MetaMask/core/pull/5756)) + ## [0.11.0] ### Changed diff --git a/packages/multichain-transactions-controller/src/MultichainTransactionsController.test.ts b/packages/multichain-transactions-controller/src/MultichainTransactionsController.test.ts index 6aafbdc680e..326f7d6a6a7 100644 --- a/packages/multichain-transactions-controller/src/MultichainTransactionsController.test.ts +++ b/packages/multichain-transactions-controller/src/MultichainTransactionsController.test.ts @@ -228,14 +228,13 @@ describe('MultichainTransactionsController', () => { await waitForAllPromises(); - expect(controller.state).toStrictEqual({ - nonEvmTransactions: { - [mockBtcAccount.id]: { - transactions: mockTransactionResult.data, - next: null, - lastUpdated: expect.any(Number), - }, - }, + const { chain } = mockTransactionResult.data[0]; + expect( + controller.state.nonEvmTransactions[mockBtcAccount.id][chain], + ).toStrictEqual({ + transactions: mockTransactionResult.data, + next: null, + lastUpdated: expect.any(Number), }); }); @@ -244,22 +243,20 @@ describe('MultichainTransactionsController', () => { setupController(); await controller.updateTransactionsForAccount(mockBtcAccount.id); - expect(controller.state).toStrictEqual({ - nonEvmTransactions: { - [mockBtcAccount.id]: { - transactions: mockTransactionResult.data, - next: null, - lastUpdated: expect.any(Number), - }, - }, + + const { chain } = mockTransactionResult.data[0]; + expect( + controller.state.nonEvmTransactions[mockBtcAccount.id][chain], + ).toStrictEqual({ + transactions: mockTransactionResult.data, + next: null, + lastUpdated: expect.any(Number), }); messenger.publish('AccountsController:accountRemoved', mockBtcAccount.id); mockListMultichainAccounts.mockReturnValue([]); - expect(controller.state).toStrictEqual({ - nonEvmTransactions: {}, - }); + expect(controller.state.nonEvmTransactions).toStrictEqual({}); }); it('does not track balances for EVM accounts', async () => { @@ -282,8 +279,9 @@ describe('MultichainTransactionsController', () => { const { controller } = setupController(); await controller.updateTransactionsForAccount(mockBtcAccount.id); + const { chain } = mockTransactionResult.data[0]; expect( - controller.state.nonEvmTransactions[mockBtcAccount.id], + controller.state.nonEvmTransactions[mockBtcAccount.id][chain], ).toStrictEqual({ transactions: mockTransactionResult.data, next: null, @@ -291,7 +289,7 @@ describe('MultichainTransactionsController', () => { }); }); - it('filters out non-mainnet Solana transactions', async () => { + it('stores transactions by chain for accounts', async () => { const mockSolTransaction = { account: mockSolAccount.id, type: 'send' as const, @@ -327,10 +325,6 @@ describe('MultichainTransactionsController', () => { ], next: null, }; - // First transaction must be the mainnet one (for the test), so we assert this. - expect(mockSolTransactions.data[0].chain).toStrictEqual( - MultichainNetwork.Solana, - ); const { controller, mockSnapHandleRequest } = setupController({ mocks: { @@ -341,10 +335,42 @@ describe('MultichainTransactionsController', () => { await controller.updateTransactionsForAccount(mockSolAccount.id); - const { transactions } = - controller.state.nonEvmTransactions[mockSolAccount.id]; - expect(transactions).toHaveLength(1); - expect(transactions[0]).toStrictEqual(mockSolTransactions.data[0]); // First transaction is the mainnet one. + expect( + Object.keys(controller.state.nonEvmTransactions[mockSolAccount.id]), + ).toHaveLength(4); + + expect( + controller.state.nonEvmTransactions[mockSolAccount.id][ + MultichainNetwork.Solana + ].transactions, + ).toHaveLength(1); + expect( + controller.state.nonEvmTransactions[mockSolAccount.id][ + MultichainNetwork.Solana + ].transactions[0], + ).toStrictEqual(mockSolTransactions.data[0]); + + expect( + controller.state.nonEvmTransactions[mockSolAccount.id][ + MultichainNetwork.SolanaTestnet + ].transactions, + ).toHaveLength(1); + expect( + controller.state.nonEvmTransactions[mockSolAccount.id][ + MultichainNetwork.SolanaTestnet + ].transactions[0], + ).toStrictEqual(mockSolTransactions.data[1]); + + expect( + controller.state.nonEvmTransactions[mockSolAccount.id][ + MultichainNetwork.SolanaDevnet + ].transactions, + ).toHaveLength(1); + expect( + controller.state.nonEvmTransactions[mockSolAccount.id][ + MultichainNetwork.SolanaDevnet + ].transactions[0], + ).toStrictEqual(mockSolTransactions.data[2]); }); it('handles pagination when fetching transactions', async () => { @@ -455,31 +481,37 @@ describe('MultichainTransactionsController', () => { id: TEST_ACCOUNT_ID, }; + const { chain } = mockTransactionResult.data[0]; const existingTransaction = { ...mockTransactionResult.data[0], id: '123', status: 'confirmed' as const, + chain, }; const newTransaction = { ...mockTransactionResult.data[0], id: '456', status: 'submitted' as const, + chain, }; const updatedExistingTransaction = { ...mockTransactionResult.data[0], id: '123', status: 'failed' as const, + chain, }; const { controller, messenger } = setupController({ state: { nonEvmTransactions: { [mockSolAccountWithId.id]: { - transactions: [existingTransaction], - next: null, - lastUpdated: Date.now(), + [chain]: { + transactions: [existingTransaction], + next: null, + lastUpdated: Date.now(), + }, }, }, }, @@ -494,7 +526,8 @@ describe('MultichainTransactionsController', () => { await waitForAllPromises(); const finalTransactions = - controller.state.nonEvmTransactions[mockSolAccountWithId.id].transactions; + controller.state.nonEvmTransactions[mockSolAccountWithId.id][chain] + .transactions; expect(finalTransactions).toStrictEqual([ updatedExistingTransaction, newTransaction, @@ -502,13 +535,16 @@ describe('MultichainTransactionsController', () => { }); it('handles empty transaction updates gracefully', async () => { + const { chain } = mockTransactionResult.data[0]; const { controller, messenger } = setupController({ state: { nonEvmTransactions: { [TEST_ACCOUNT_ID]: { - transactions: [], - next: null, - lastUpdated: Date.now(), + [chain]: { + transactions: [], + next: null, + lastUpdated: Date.now(), + }, }, }, }, @@ -520,7 +556,9 @@ describe('MultichainTransactionsController', () => { await waitForAllPromises(); - expect(controller.state.nonEvmTransactions[TEST_ACCOUNT_ID]).toStrictEqual({ + expect( + controller.state.nonEvmTransactions[TEST_ACCOUNT_ID][chain], + ).toStrictEqual({ transactions: [], next: null, lastUpdated: expect.any(Number), @@ -528,6 +566,8 @@ describe('MultichainTransactionsController', () => { }); it('initializes new accounts with empty transactions array when receiving updates', async () => { + const { chain } = mockTransactionResult.data[0]; + const { controller, messenger } = setupController({ state: { nonEvmTransactions: {}, @@ -541,21 +581,26 @@ describe('MultichainTransactionsController', () => { }); await waitForAllPromises(); - - expect(controller.state.nonEvmTransactions[NEW_ACCOUNT_ID]).toStrictEqual({ + expect( + controller.state.nonEvmTransactions[NEW_ACCOUNT_ID][chain], + ).toStrictEqual({ transactions: mockTransactionResult.data, + next: null, lastUpdated: expect.any(Number), }); }); it('handles undefined transactions in update payload', async () => { + const { chain } = mockTransactionResult.data[0]; const { controller, messenger } = setupController({ state: { nonEvmTransactions: { [TEST_ACCOUNT_ID]: { - transactions: [], - next: null, - lastUpdated: Date.now(), + [chain]: { + transactions: [], + next: null, + lastUpdated: Date.now(), + }, }, }, }, @@ -570,8 +615,10 @@ describe('MultichainTransactionsController', () => { const initialStateSnapshot = { [TEST_ACCOUNT_ID]: { - ...controller.state.nonEvmTransactions[TEST_ACCOUNT_ID], - lastUpdated: expect.any(Number), + [chain]: { + ...controller.state.nonEvmTransactions[TEST_ACCOUNT_ID][chain], + lastUpdated: expect.any(Number), + }, }, }; @@ -587,6 +634,7 @@ describe('MultichainTransactionsController', () => { }); it('sorts transactions by timestamp (newest first)', async () => { + const { chain } = mockTransactionResult.data[0]; const olderTransaction = { ...mockTransactionResult.data[0], id: '123', @@ -602,9 +650,11 @@ describe('MultichainTransactionsController', () => { state: { nonEvmTransactions: { [TEST_ACCOUNT_ID]: { - transactions: [olderTransaction], - next: null, - lastUpdated: Date.now(), + [chain]: { + transactions: [olderTransaction], + next: null, + lastUpdated: Date.now(), + }, }, }, }, @@ -619,7 +669,7 @@ describe('MultichainTransactionsController', () => { await waitForAllPromises(); const finalTransactions = - controller.state.nonEvmTransactions[TEST_ACCOUNT_ID].transactions; + controller.state.nonEvmTransactions[TEST_ACCOUNT_ID][chain].transactions; expect(finalTransactions).toStrictEqual([ newerTransaction, olderTransaction, @@ -627,6 +677,7 @@ describe('MultichainTransactionsController', () => { }); it('sorts transactions by timestamp and handles null timestamps', async () => { + const { chain } = mockTransactionResult.data[0]; const nullTimestampTx1 = { ...mockTransactionResult.data[0], id: '123', @@ -647,9 +698,11 @@ describe('MultichainTransactionsController', () => { state: { nonEvmTransactions: { [TEST_ACCOUNT_ID]: { - transactions: [nullTimestampTx1], - next: null, - lastUpdated: Date.now(), + [chain]: { + transactions: [nullTimestampTx1], + next: null, + lastUpdated: Date.now(), + }, }, }, }, @@ -664,7 +717,7 @@ describe('MultichainTransactionsController', () => { await waitForAllPromises(); const finalTransactions = - controller.state.nonEvmTransactions[TEST_ACCOUNT_ID].transactions; + controller.state.nonEvmTransactions[TEST_ACCOUNT_ID][chain].transactions; expect(finalTransactions).toStrictEqual([ withTimestampTx, nullTimestampTx1, @@ -685,8 +738,10 @@ describe('MultichainTransactionsController', () => { mockGetKeyringState.mockReturnValue({ isUnlocked: true }); await controller.updateTransactionsForAccount(mockBtcAccount.id); + + const { chain } = mockTransactionResult.data[0]; expect( - controller.state.nonEvmTransactions[mockBtcAccount.id], + controller.state.nonEvmTransactions[mockBtcAccount.id][chain], ).toStrictEqual({ transactions: mockTransactionResult.data, next: null, @@ -694,7 +749,7 @@ describe('MultichainTransactionsController', () => { }); }); - it('filters out non-mainnet Solana transactions in transaction updates', async () => { + it('updates transactions by chain when receiving transaction updates', async () => { const mockSolAccountWithId = { ...mockSolAccount, id: TEST_ACCOUNT_ID, @@ -732,9 +787,11 @@ describe('MultichainTransactionsController', () => { state: { nonEvmTransactions: { [mockSolAccountWithId.id]: { - transactions: [], - next: null, - lastUpdated: Date.now(), + [MultichainNetwork.Solana]: { + transactions: [], + next: null, + lastUpdated: Date.now(), + }, }, }, }, @@ -748,11 +805,31 @@ describe('MultichainTransactionsController', () => { await waitForAllPromises(); - const finalTransactions = - controller.state.nonEvmTransactions[mockSolAccountWithId.id].transactions; + expect( + Object.keys(controller.state.nonEvmTransactions[mockSolAccountWithId.id]), + ).toHaveLength(2); - expect(finalTransactions).toHaveLength(1); - expect(finalTransactions[0]).toBe(mainnetTransaction); + expect( + controller.state.nonEvmTransactions[mockSolAccountWithId.id][ + MultichainNetwork.Solana + ].transactions, + ).toHaveLength(1); + expect( + controller.state.nonEvmTransactions[mockSolAccountWithId.id][ + MultichainNetwork.Solana + ].transactions[0], + ).toBe(mainnetTransaction); + + expect( + controller.state.nonEvmTransactions[mockSolAccountWithId.id][ + MultichainNetwork.SolanaDevnet + ].transactions, + ).toHaveLength(1); + expect( + controller.state.nonEvmTransactions[mockSolAccountWithId.id][ + MultichainNetwork.SolanaDevnet + ].transactions[0], + ).toBe(devnetTransaction); }); it('publishes transactionConfirmed event when transaction is confirmed', async () => { diff --git a/packages/multichain-transactions-controller/src/MultichainTransactionsController.ts b/packages/multichain-transactions-controller/src/MultichainTransactionsController.ts index fc8b09b239a..b36b3c667e3 100644 --- a/packages/multichain-transactions-controller/src/MultichainTransactionsController.ts +++ b/packages/multichain-transactions-controller/src/MultichainTransactionsController.ts @@ -23,15 +23,12 @@ import type { HandleSnapRequest } from '@metamask/snaps-controllers'; import type { SnapId } from '@metamask/snaps-sdk'; import { HandlerType } from '@metamask/snaps-utils'; import { - KnownCaipNamespace, - parseCaipChainId, + type CaipChainId, type Json, type JsonRpcRequest, } from '@metamask/utils'; import type { Draft } from 'immer'; -import { MultichainNetwork } from './constants'; - const controllerName = 'MultichainTransactionsController'; /** @@ -51,7 +48,9 @@ export type PaginationOptions = { */ export type MultichainTransactionsControllerState = { nonEvmTransactions: { - [accountId: string]: TransactionStateEntry; + [accountId: string]: { + [chain: CaipChainId]: TransactionStateEntry; + }; }; }; @@ -156,7 +155,7 @@ const multichainTransactionsControllerMetadata = { }; /** - * The state of transactions for a specific account. + * The state of transactions for a specific chain. */ export type TransactionStateEntry = { transactions: Transaction[]; @@ -285,16 +284,36 @@ export class MultichainTransactionsController extends BaseController< { limit: 10 }, ); - const transactions = this.#filterTransactions(response.data); + const transactionsByChain: Record = {}; + + response.data.forEach((transaction) => { + const { chain } = transaction; + + if (!transactionsByChain[chain]) { + transactionsByChain[chain] = []; + } + transactionsByChain[chain].push(transaction); + }); + + const chainUpdates = Object.entries(transactionsByChain).map( + ([chain, transactions]) => ({ + chain, + entry: { + transactions, + next: response.next, + lastUpdated: Date.now(), + }, + }), + ); this.update((state: Draft) => { - const entry: TransactionStateEntry = { - transactions, - next: response.next, - lastUpdated: Date.now(), - }; + if (!state.nonEvmTransactions[account.id]) { + state.nonEvmTransactions[account.id] = {}; + } - Object.assign(state.nonEvmTransactions, { [account.id]: entry }); + chainUpdates.forEach(({ chain, entry }) => { + state.nonEvmTransactions[account.id][chain as CaipChainId] = entry; + }); }); } } catch (error) { @@ -305,27 +324,6 @@ export class MultichainTransactionsController extends BaseController< } } - /** - * Filters transactions to only include mainnet Solana transactions for Solana chains. - * Non-Solana chain transactions are kept as is. - * - * @param transactions - Array of transactions to filter - * @returns Filtered transactions array - */ - #filterTransactions(transactions: Transaction[]): Transaction[] { - return transactions.filter((tx) => { - const chain = tx.chain as MultichainNetwork; - const { namespace } = parseCaipChainId(chain); - - // Enum comparison is safe here as we control both enum values - // eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison - if (namespace === KnownCaipNamespace.Solana) { - return chain === MultichainNetwork.Solana; - } - return true; - }); - } - /** * Checks for non-EVM accounts. * @@ -395,7 +393,10 @@ export class MultichainTransactionsController extends BaseController< #handleOnAccountTransactionsUpdated( transactionsUpdate: AccountTransactionsUpdatedEventPayload, ): void { - const updatedTransactions: Record = {}; + const updatedTransactions: Record< + string, + Record + > = {}; const transactionsToPublish: Transaction[] = []; if (!transactionsUpdate?.transactions) { @@ -404,45 +405,63 @@ export class MultichainTransactionsController extends BaseController< Object.entries(transactionsUpdate.transactions).forEach( ([accountId, newTransactions]) => { - // Account might not have any transactions yet, so use `[]` in that case. - const oldTransactions = - this.state.nonEvmTransactions[accountId]?.transactions ?? []; + updatedTransactions[accountId] = {}; - const filteredNewTransactions = - this.#filterTransactions(newTransactions); + newTransactions.forEach((tx) => { + const { chain } = tx; - // Uses a `Map` to deduplicate transactions by ID, ensuring we keep the latest version - // of each transaction while preserving older transactions and transactions from other accounts. - // Transactions are sorted by timestamp (newest first). - const transactions = new Map(); + if (!updatedTransactions[accountId][chain]) { + updatedTransactions[accountId][chain] = []; + } - oldTransactions.forEach((tx) => { - transactions.set(tx.id, tx); - }); - - filteredNewTransactions.forEach((tx) => { - transactions.set(tx.id, tx); + updatedTransactions[accountId][chain].push(tx); transactionsToPublish.push(tx); }); - // Sorted by timestamp (newest first). If the timestamp is not provided, those - // transactions will be put in the end of this list. - updatedTransactions[accountId] = Array.from(transactions.values()).sort( - (a, b) => (b.timestamp ?? 0) - (a.timestamp ?? 0), + Object.entries(updatedTransactions[accountId]).forEach( + ([chain, chainTransactions]) => { + // Account might not have any transactions yet, so use `[]` in that case. + const oldTransactions = + this.state.nonEvmTransactions[accountId]?.[chain as CaipChainId] + ?.transactions ?? []; + + // Uses a `Map` to deduplicate transactions by ID, ensuring we keep the latest version + // of each transaction while preserving older transactions and transactions from other accounts. + // Transactions are sorted by timestamp (newest first). + const transactions = new Map(); + + oldTransactions.forEach((tx) => { + transactions.set(tx.id, tx); + }); + + chainTransactions.forEach((tx) => { + transactions.set(tx.id, tx); + }); + + // Sorted by timestamp (newest first). If the timestamp is not provided, those + // transactions will be put in the end of this list. + updatedTransactions[accountId][chain as CaipChainId] = Array.from( + transactions.values(), + ).sort((a, b) => (b.timestamp ?? 0) - (a.timestamp ?? 0)); + }, ); }, ); this.update((state) => { - Object.entries(updatedTransactions).forEach( - ([accountId, transactions]) => { - state.nonEvmTransactions[accountId] = { - ...state.nonEvmTransactions[accountId], + Object.entries(updatedTransactions).forEach(([accountId, chainsData]) => { + if (!state.nonEvmTransactions[accountId]) { + state.nonEvmTransactions[accountId] = {}; + } + + Object.entries(chainsData).forEach(([chain, transactions]) => { + state.nonEvmTransactions[accountId][chain as CaipChainId] = { transactions, + next: null, lastUpdated: Date.now(), }; - }, - ); + }); + }); }); // After we update the state, publish the events for new/updated transactions