diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 988e9ac5eba..fdcee545726 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `perpsAcrossDeposit` and `predictAcrossDeposit` transaction types for Across MetaMask Pay submissions ([#7886](https://github.com/MetaMask/core/pull/7886)) +### Fixed + +- Use effective recipient (decoded from tx data for token transfers) when checking for existing transactions in first-time interaction logic ([#8130](https://github.com/MetaMask/core/pull/8130)) + ## [62.20.0] ### Changed diff --git a/packages/transaction-controller/src/utils/first-time-interaction.test.ts b/packages/transaction-controller/src/utils/first-time-interaction.test.ts index 4ea633e7f96..ae130da6580 100644 --- a/packages/transaction-controller/src/utils/first-time-interaction.test.ts +++ b/packages/transaction-controller/src/utils/first-time-interaction.test.ts @@ -300,6 +300,108 @@ describe('updateFirstTimeInteraction', () => { expect(mockGetAccountAddressRelationship).toHaveBeenCalled(); expect(mockUpdateTransactionInternal).toHaveBeenCalled(); }); + + it('proceeds for ERC20 when existing tx has same contract (txParams.to) but different decoded recipient', async () => { + const tokenContract = '0xcontract'; + const currentRecipient = '0xrecipientA'; + const otherRecipient = '0xrecipientB'; + + const currentTx: TransactionMeta = { + ...mockTransactionMeta, + id: 'current-tx', + type: TransactionType.tokenMethodTransfer, + txParams: { + ...mockTransactionMeta.txParams, + to: tokenContract, + data: '0xdata', + }, + } as unknown as TransactionMeta; + + const existingTxSameContract: TransactionMeta = { + ...mockTransactionMeta, + id: 'existing-tx', + type: TransactionType.tokenMethodTransfer, + txParams: { + ...mockTransactionMeta.txParams, + from: '0xfrom', + to: tokenContract, + data: '0xother', + }, + } as unknown as TransactionMeta; + + mockDecodeTransactionData + .mockReturnValueOnce({ + name: 'transfer', + args: { _to: currentRecipient }, + } as unknown as ReturnType) + .mockReturnValueOnce({ + name: 'transfer', + args: { _to: otherRecipient }, + } as unknown as ReturnType); + + mockGetAccountAddressRelationship.mockResolvedValue({ count: 0 }); + + await updateFirstTimeInteraction({ + existingTransactions: [existingTxSameContract], + getTransaction: mockGetTransaction, + isFirstTimeInteractionEnabled: mockIsFirstTimeInteractionEnabled, + trace: mockTrace, + transactionMeta: currentTx, + updateTransaction: mockUpdateTransactionInternal, + }); + + expect(mockGetAccountAddressRelationship).toHaveBeenCalledWith({ + chainId: 1, + to: currentRecipient, + from: '0xfrom', + }); + expect(mockUpdateTransactionInternal).toHaveBeenCalled(); + }); + + it('returns early for ERC20 when existing tx has same effective recipient (decoded)', async () => { + const tokenContract = '0xcontract'; + const recipient = '0xrecipientA'; + + const currentTx: TransactionMeta = { + ...mockTransactionMeta, + id: 'current-tx', + type: TransactionType.tokenMethodTransfer, + txParams: { + ...mockTransactionMeta.txParams, + to: tokenContract, + data: '0xdata', + }, + } as unknown as TransactionMeta; + + const existingTxSameRecipient: TransactionMeta = { + ...mockTransactionMeta, + id: 'existing-tx', + type: TransactionType.tokenMethodTransfer, + txParams: { + ...mockTransactionMeta.txParams, + from: '0xfrom', + to: tokenContract, + data: '0xother', + }, + } as unknown as TransactionMeta; + + mockDecodeTransactionData.mockReturnValue({ + name: 'transfer', + args: { _to: recipient }, + } as unknown as ReturnType); + + await updateFirstTimeInteraction({ + existingTransactions: [existingTxSameRecipient], + getTransaction: mockGetTransaction, + isFirstTimeInteractionEnabled: mockIsFirstTimeInteractionEnabled, + trace: mockTrace, + transactionMeta: currentTx, + updateTransaction: mockUpdateTransactionInternal, + }); + + expect(mockGetAccountAddressRelationship).not.toHaveBeenCalled(); + expect(mockUpdateTransactionInternal).not.toHaveBeenCalled(); + }); }); describe('API integration', () => { diff --git a/packages/transaction-controller/src/utils/first-time-interaction.ts b/packages/transaction-controller/src/utils/first-time-interaction.ts index 32fa4fe90ec..b9546d4d978 100644 --- a/packages/transaction-controller/src/utils/first-time-interaction.ts +++ b/packages/transaction-controller/src/utils/first-time-interaction.ts @@ -10,6 +10,27 @@ import { projectLogger as log } from '../logger'; import { TransactionType } from '../types'; import type { TransactionMeta } from '../types'; +const TOKEN_TRANSFER_TYPES = [ + TransactionType.tokenMethodTransfer, + TransactionType.tokenMethodTransferFrom, +]; + +/** + * Returns the effective recipient for first-time-interaction checks (decoded from data for token transfers). + * Used when comparing existing transactions so we match by actual recipient, not txParams.to (contract for ERC20). + * + * @param tx - Transaction meta with txParams and type + * @returns Effective recipient address, or undefined + */ +function getEffectiveRecipient(tx: TransactionMeta): string | undefined { + const { data, to } = tx?.txParams ?? {}; + if (data && TOKEN_TRANSFER_TYPES.includes(tx?.type as TransactionType)) { + const parsed = decodeTransactionData(data) as TransactionDescription; + return (parsed?.args?._to ?? parsed?.args?.to ?? to) as string | undefined; + } + return to; +} + type UpdateFirstTimeInteractionRequest = { existingTransactions: TransactionMeta[]; getTransaction: (transactionId: string) => TransactionMeta | undefined; @@ -60,13 +81,7 @@ export async function updateFirstTimeInteraction({ } = transactionMeta; let recipient; - if ( - data && - [ - TransactionType.tokenMethodTransfer, - TransactionType.tokenMethodTransferFrom, - ].includes(type as TransactionType) - ) { + if (data && TOKEN_TRANSFER_TYPES.includes(type as TransactionType)) { const parsedData = decodeTransactionData(data) as TransactionDescription; // _to is for ERC20, ERC721 and USDC // to is for ERC1155 @@ -84,16 +99,17 @@ export async function updateFirstTimeInteraction({ validateParamTo(recipient); + const recipientLower = recipient?.toLowerCase(); const existingTransaction = existingTransactions.find( (tx) => + tx.id !== transactionId && tx.chainId === chainId && - tx.txParams.from.toLowerCase() === from.toLowerCase() && - tx.txParams.to?.toLowerCase() === to?.toLowerCase() && - tx.id !== transactionId, + tx.txParams?.from?.toLowerCase() === from?.toLowerCase() && + getEffectiveRecipient(tx)?.toLowerCase() === recipientLower, ); - // Check if there is an existing transaction with the same from, to, and chainId - // else we continue to check the account address relationship from API + // Skip API call only if we already have a tx with same from and same effective recipient (e.g. duplicate or pending). + // For ERC20, effective recipient is decoded from data; using txParams.to would wrongly match any send of that token. if (existingTransaction) { return; }