Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/transaction-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof decodeTransactionData>)
.mockReturnValueOnce({
name: 'transfer',
args: { _to: otherRecipient },
} as unknown as ReturnType<typeof decodeTransactionData>);

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<typeof decodeTransactionData>);

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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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;
}
Expand Down
Loading