diff --git a/packages/snap/snap.manifest.json b/packages/snap/snap.manifest.json index 55319d42..f830dc71 100644 --- a/packages/snap/snap.manifest.json +++ b/packages/snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snap-tron-wallet.git" }, "source": { - "shasum": "U77pFvmwZo1AFVSfIbwNNEFOjDUgmMRv2EPP0a66lBI=", + "shasum": "Qzmvl2UHLOcowpF1G750VpKfM7/rTWccsHL4hD/ZJqI=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snap/src/services/transactions/TransactionsRepository.test.ts b/packages/snap/src/services/transactions/TransactionsRepository.test.ts new file mode 100644 index 00000000..10deeb79 --- /dev/null +++ b/packages/snap/src/services/transactions/TransactionsRepository.test.ts @@ -0,0 +1,144 @@ +import type { Transaction } from '@metamask/keyring-api'; +import { TransactionStatus, TransactionType } from '@metamask/keyring-api'; + +import { TransactionsRepository } from './TransactionsRepository'; +import { KnownCaip19Id, Network } from '../../constants'; +import type { State, UnencryptedStateValue } from '../state/State'; + +describe('TransactionsRepository', () => { + let transactionsRepository: TransactionsRepository; + let mockState: jest.Mocked>; + + const mockAccountId = 'test-account-id'; + + const createMockTransaction = ( + id: string, + status: TransactionStatus, + ): Transaction => ({ + id, + type: TransactionType.Send, + account: mockAccountId, + chain: Network.Mainnet, + status, + timestamp: Math.floor(Date.now() / 1000), + from: [ + { + address: 'TGJn1wnUYHJbvN88cynZbsAz2EMeZq73yx', + asset: { + type: KnownCaip19Id.TrxMainnet, + amount: '100', + unit: 'TRX', + fungible: true, + }, + }, + ], + to: [ + { + address: 'TFDP1vFeSYPT6FUznL7zUjhg5X7p2AA8vw', + asset: { + type: KnownCaip19Id.TrxMainnet, + amount: '100', + unit: 'TRX', + fungible: true, + }, + }, + ], + events: [{ status, timestamp: Math.floor(Date.now() / 1000) }], + fees: [], + }); + + beforeEach(() => { + mockState = { + getKey: jest.fn(), + setKey: jest.fn(), + update: jest.fn(), + } as unknown as jest.Mocked>; + + transactionsRepository = new TransactionsRepository(mockState); + }); + + describe('getTransactionIdsByAccountId', () => { + it('returns all transaction IDs for an account', async () => { + const transactions = [ + createMockTransaction('tx1', TransactionStatus.Confirmed), + createMockTransaction('tx2', TransactionStatus.Unconfirmed), + createMockTransaction('tx3', TransactionStatus.Failed), + ]; + mockState.getKey.mockResolvedValue(transactions); + + const result = + await transactionsRepository.getTransactionIdsByAccountId( + mockAccountId, + ); + + expect(result).toStrictEqual(new Set(['tx1', 'tx2', 'tx3'])); + }); + + it('returns empty set when no transactions exist', async () => { + mockState.getKey.mockResolvedValue(null); + + const result = + await transactionsRepository.getTransactionIdsByAccountId( + mockAccountId, + ); + + expect(result).toStrictEqual(new Set()); + }); + }); + + describe('getConfirmedTransactionIds', () => { + it('returns only confirmed and failed transaction IDs, excluding pending', async () => { + const transactions = [ + createMockTransaction('confirmed-tx', TransactionStatus.Confirmed), + createMockTransaction('pending-tx', TransactionStatus.Unconfirmed), + createMockTransaction('failed-tx', TransactionStatus.Failed), + ]; + mockState.getKey.mockResolvedValue(transactions); + + const result = + await transactionsRepository.getConfirmedTransactionIds(mockAccountId); + + // Should include confirmed and failed, but NOT unconfirmed (pending) + expect(result).toStrictEqual(new Set(['confirmed-tx', 'failed-tx'])); + expect(result.has('pending-tx')).toBe(false); + }); + + it('returns empty set when all transactions are pending', async () => { + const transactions = [ + createMockTransaction('pending-tx-1', TransactionStatus.Unconfirmed), + createMockTransaction('pending-tx-2', TransactionStatus.Unconfirmed), + ]; + mockState.getKey.mockResolvedValue(transactions); + + const result = + await transactionsRepository.getConfirmedTransactionIds(mockAccountId); + + expect(result).toStrictEqual(new Set()); + }); + + it('returns empty set when no transactions exist', async () => { + mockState.getKey.mockResolvedValue(null); + + const result = + await transactionsRepository.getConfirmedTransactionIds(mockAccountId); + + expect(result).toStrictEqual(new Set()); + }); + + it('returns all IDs when no transactions are pending', async () => { + const transactions = [ + createMockTransaction('confirmed-tx-1', TransactionStatus.Confirmed), + createMockTransaction('confirmed-tx-2', TransactionStatus.Confirmed), + createMockTransaction('failed-tx', TransactionStatus.Failed), + ]; + mockState.getKey.mockResolvedValue(transactions); + + const result = + await transactionsRepository.getConfirmedTransactionIds(mockAccountId); + + expect(result).toStrictEqual( + new Set(['confirmed-tx-1', 'confirmed-tx-2', 'failed-tx']), + ); + }); + }); +}); diff --git a/packages/snap/src/services/transactions/TransactionsRepository.ts b/packages/snap/src/services/transactions/TransactionsRepository.ts index ef2851c1..aec81f62 100644 --- a/packages/snap/src/services/transactions/TransactionsRepository.ts +++ b/packages/snap/src/services/transactions/TransactionsRepository.ts @@ -1,4 +1,5 @@ import type { Transaction } from '@metamask/keyring-api'; +import { TransactionStatus } from '@metamask/keyring-api'; import { chain } from 'lodash'; import type { State, UnencryptedStateValue } from '../state/State'; @@ -43,6 +44,26 @@ export class TransactionsRepository { return new Set((transactions ?? []).map((tx) => tx.id)); } + /** + * Gets transaction IDs for confirmed (non-pending) transactions only. + * This allows pending transactions to be re-fetched and updated when + * their confirmed version becomes available from the network. + * + * @param accountId - The account ID to get confirmed transaction IDs for. + * @returns Set of transaction IDs for confirmed transactions only. + */ + async getConfirmedTransactionIds(accountId: string): Promise> { + const transactions = await this.#state.getKey( + `${this.#stateKey}.${accountId}`, + ); + + return new Set( + (transactions ?? []) + .filter((tx) => tx.status !== TransactionStatus.Unconfirmed) + .map((tx) => tx.id), + ); + } + async save(transaction: Transaction): Promise { await this.saveMany([transaction]); } diff --git a/packages/snap/src/services/transactions/TransactionsService.test.ts b/packages/snap/src/services/transactions/TransactionsService.test.ts index 21f127b2..3704f369 100644 --- a/packages/snap/src/services/transactions/TransactionsService.test.ts +++ b/packages/snap/src/services/transactions/TransactionsService.test.ts @@ -71,6 +71,7 @@ describe('TransactionsService', () => { getAll: jest.fn(), findByAccountId: jest.fn().mockResolvedValue([]), getTransactionIdsByAccountId: jest.fn().mockResolvedValue(new Set()), + getConfirmedTransactionIds: jest.fn().mockResolvedValue(new Set()), save: jest.fn(), saveMany: jest.fn(), } as unknown as jest.Mocked; @@ -288,6 +289,65 @@ describe('TransactionsService', () => { ); expect(true).toBe(true); }); + + it('skips already confirmed transactions but allows pending transactions to be updated', async () => { + // Simulate a confirmed transaction ID that should be skipped + const confirmedTxId = 'confirmed-tx-id'; + mockTransactionsRepository.getConfirmedTransactionIds.mockResolvedValue( + new Set([confirmedTxId]), + ); + + // API returns both the confirmed tx and a new tx + const confirmedTx = { + ...nativeTransferMock, + txID: confirmedTxId, + }; + const newTx = { + ...nativeTransferMock, + txID: 'new-tx-id', + }; + mockTrongridApiClient.getTransactionInfoByAddress.mockResolvedValue([ + confirmedTx, + newTx, + ] as TransactionInfo[]); + mockTrongridApiClient.getContractTransactionInfoByAddress.mockResolvedValue( + [], + ); + + const result = await transactionsService.fetchNewTransactionsForAccount( + Network.Mainnet, + mockAccount, + ); + + // Should only return the new transaction, not the confirmed one + expect(result).toHaveLength(1); + expect(result[0]!.id).toBe('new-tx-id'); + }); + + it('re-fetches pending transactions so they can be updated to confirmed status', async () => { + // Simulate a pending transaction that exists in state but is NOT in confirmed set + const pendingTxId = nativeTransferMock.txID; + mockTransactionsRepository.getConfirmedTransactionIds.mockResolvedValue( + new Set(), // Pending tx ID is not in confirmed set + ); + + // API returns the same transaction (now confirmed on network) + mockTrongridApiClient.getTransactionInfoByAddress.mockResolvedValue([ + nativeTransferMock, + ] as TransactionInfo[]); + mockTrongridApiClient.getContractTransactionInfoByAddress.mockResolvedValue( + [], + ); + + const result = await transactionsService.fetchNewTransactionsForAccount( + Network.Mainnet, + mockAccount, + ); + + // The pending transaction should be returned so it can be updated + expect(result).toHaveLength(1); + expect(result[0]!.id).toBe(pendingTxId); + }); }); describe('findByAccounts', () => { diff --git a/packages/snap/src/services/transactions/TransactionsService.ts b/packages/snap/src/services/transactions/TransactionsService.ts index 6c9937b5..9828e995 100644 --- a/packages/snap/src/services/transactions/TransactionsService.ts +++ b/packages/snap/src/services/transactions/TransactionsService.ts @@ -198,17 +198,17 @@ export class TransactionsService { ); /** - * Step 1: Load existing transaction IDs from state + * Step 1: Load confirmed transaction IDs from state * - * We only need the IDs to check which transactions are new. + * We only skip transactions that are already confirmed. Pending (Unconfirmed) + * transactions are allowed to be re-fetched so their status can be updated + * when they become confirmed on the network. */ - const existingTxIds = - await this.#transactionsRepository.getTransactionIdsByAccountId( - account.id, - ); + const confirmedTxIds = + await this.#transactionsRepository.getConfirmedTransactionIds(account.id); this.#logger.debug( - `Found ${existingTxIds.size} existing transactions in state for account ${account.id}.`, + `Found ${confirmedTxIds.size} confirmed transactions in state for account ${account.id}.`, ); /** @@ -226,53 +226,54 @@ export class TransactionsService { ); /** - * Step 3: Filter to only NEW transactions + * Step 3: Filter to transactions that need processing * - * Skip transactions that are already processed and saved in state. - * This is the core optimization - we avoid re-processing known transactions. + * Skip transactions that are already confirmed in state. + * Pending transactions are included so they can be updated when confirmed. + * This maintains the optimization while allowing status updates. */ - const newRawTransactions = rawTransactions.filter( - (tx) => !existingTxIds.has(tx.txID), + const transactionsToProcess = rawTransactions.filter( + (tx) => !confirmedTxIds.has(tx.txID), ); this.#logger.info( - `Found ${newRawTransactions.length} new transactions to process (${rawTransactions.length - newRawTransactions.length} already in state).`, + `Found ${transactionsToProcess.length} transactions to process (${rawTransactions.length - transactionsToProcess.length} already confirmed in state).`, ); /** - * Step 4: If no new transactions, return empty array + * Step 4: If no transactions to process, return empty array * * This is the fast path - skip all enrichment and mapping. */ - if (newRawTransactions.length === 0) { - this.#logger.info(`No new transactions found.`); + if (transactionsToProcess.length === 0) { + this.#logger.info(`No transactions to process.`); return []; } /** - * Step 5: Process only NEW transactions (enrichment + mapping) + * Step 5: Process transactions (enrichment + mapping) * * Enrich potential swaps with internal_transactions data - * and fetch TRC10 token metadata for new transactions only. + * and fetch TRC10 token metadata for transactions to process. */ const { enrichedRawTransactions, trc10TokenMetadata } = - await this.#fetchEnrichmentData(scope, newRawTransactions); + await this.#fetchEnrichmentData(scope, transactionsToProcess); this.#logger.info( - `Enriched ${enrichedRawTransactions.length} new transactions, fetched metadata for ${trc10TokenMetadata.size} TRC10 tokens.`, + `Enriched ${enrichedRawTransactions.length} transactions, fetched metadata for ${trc10TokenMetadata.size} TRC10 tokens.`, ); /** - * Step 6: Map new transactions to keyring format + * Step 6: Map transactions to keyring format * - * Filter TRC20 assistance data to only include transactions matching new raw txs. + * Filter TRC20 assistance data to only include transactions matching processed txs. */ - const newTxIds = new Set(newRawTransactions.map((tx) => tx.txID)); + const processedTxIds = new Set(transactionsToProcess.map((tx) => tx.txID)); const relevantTrc20Transactions = trc20Transactions.filter((tx) => - newTxIds.has(tx.transaction_id), + processedTxIds.has(tx.transaction_id), ); - const newMappedTransactions = TransactionMapper.mapTransactions({ + const mappedTransactions = TransactionMapper.mapTransactions({ scope, account: account as TronKeyringAccount, rawTransactions: enrichedRawTransactions, @@ -281,10 +282,10 @@ export class TransactionsService { }); this.#logger.info( - `Returning ${newMappedTransactions.length} new transactions for account ${account.address} on network ${scope}.`, + `Returning ${mappedTransactions.length} transactions for account ${account.address} on network ${scope}.`, ); - return newMappedTransactions; + return mappedTransactions; } /**