feat: add incoming transaction types (#5987)

## Explanation

Determine the specific `TransactionType` for outgoing transactions
retrieved from accounts API.

## References

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs),
highlighting breaking changes as necessary
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes

Author: matthewwalsh0

packages/transaction-controller/CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add specific transaction types to outgoing transactions retrieved from accounts API ([#5987](https://github.com/MetaMask/core/pull/5987))
+  - Add optional `amount` property to `transferInformation` object in `TransactionMeta` type.
+
 ### Changed
 
 - Remove incoming transactions when calling `wipeTransactions` ([#5986](https://github.com/MetaMask/core/pull/5986))

packages/transaction-controller/src/api/accounts-api.ts

@@ -59,7 +59,7 @@ export type TransactionResponse = {
   effectiveGasPrice: string;
   nonce: number;
   cumulativeGasUsed: number;
-  methodId: null;
+  methodId?: Hex;
   value: string;
   to: string;
   from: string;

packages/transaction-controller/src/helpers/AccountsApiRemoteTransactionSource.test.ts

@@ -2,14 +2,16 @@ import {
   AccountsApiRemoteTransactionSource,
   SUPPORTED_CHAIN_IDS,
 } from './AccountsApiRemoteTransactionSource';
+import { determineTransactionType } from '..';
 import type {
   GetAccountTransactionsResponse,
   TransactionResponse,
 } from '../api/accounts-api';
 import { getAccountTransactions } from '../api/accounts-api';
-import type { RemoteTransactionSourceRequest } from '../types';
+import { TransactionType, type RemoteTransactionSourceRequest } from '../types';
 
 jest.mock('../api/accounts-api');
+jest.mock('../utils/transaction-type');
 
 jest.useFakeTimers();
 
@@ -41,7 +43,7 @@ const RESPONSE_STANDARD_MOCK: TransactionResponse = {
   effectiveGasPrice: '1',
   nonce: 1,
   cumulativeGasUsed: 1,
-  methodId: null,
+  methodId: '0x12345678',
   value: '1',
   to: ADDRESS_MOCK,
   from: '0x2',
@@ -78,6 +80,7 @@ const TRANSACTION_STANDARD_MOCK = {
   transferInformation: undefined,
   txParams: {
     chainId: '0x1',
+    data: '0x12345678',
     from: '0x2',
     gas: '0x1',
     gasPrice: '0x1',
@@ -86,14 +89,15 @@ const TRANSACTION_STANDARD_MOCK = {
     to: '0x123',
     value: '0x1',
   },
-  type: 'incoming',
+  type: TransactionType.incoming,
   verifiedOnBlockchain: false,
 };
 
 const TRANSACTION_TOKEN_TRANSFER_MOCK = {
   ...TRANSACTION_STANDARD_MOCK,
   isTransfer: true,
   transferInformation: {
+    amount: '1',
     contractAddress: '0x123',
     decimals: 18,
     symbol: 'ABC',
@@ -102,6 +106,7 @@ const TRANSACTION_TOKEN_TRANSFER_MOCK = {
 
 describe('AccountsApiRemoteTransactionSource', () => {
   const getAccountTransactionsMock = jest.mocked(getAccountTransactions);
+  const determineTransactionTypeMock = jest.mocked(determineTransactionType);
 
   beforeEach(() => {
     jest.resetAllMocks();
@@ -110,6 +115,11 @@ describe('AccountsApiRemoteTransactionSource', () => {
     getAccountTransactionsMock.mockResolvedValue(
       {} as GetAccountTransactionsResponse,
     );
+
+    determineTransactionTypeMock.mockResolvedValue({
+      type: TransactionType.tokenMethodTransfer,
+      getCodeResponse: undefined,
+    });
   });
 
   describe('getSupportedChains', () => {
@@ -352,5 +362,19 @@ describe('AccountsApiRemoteTransactionSource', () => {
 
       expect(transactions).toStrictEqual([]);
     });
+
+    it('determines transaction type if outgoing', async () => {
+      getAccountTransactionsMock.mockResolvedValue({
+        data: [{ ...RESPONSE_TOKEN_TRANSFER_MOCK, from: ADDRESS_MOCK }],
+        pageInfo: { hasNextPage: false, count: 1 },
+      });
+
+      const transactions =
+        await new AccountsApiRemoteTransactionSource().fetchTransactions(
+          REQUEST_MOCK,
+        );
+
+      expect(transactions[0].type).toBe(TransactionType.tokenMethodTransfer);
+    });
   });
 });

packages/transaction-controller/src/helpers/AccountsApiRemoteTransactionSource.ts

@@ -3,6 +3,7 @@ import type { Hex } from '@metamask/utils';
 import BN from 'bn.js';
 import { v1 as random } from 'uuid';
 
+import { determineTransactionType } from '..';
 import type {
   GetAccountTransactionsResponse,
   TransactionResponse,
@@ -60,8 +61,8 @@ export class AccountsApiRemoteTransactionSource
       responseTransactions,
     );
 
-    const normalizedTransactions = responseTransactions.map((tx) =>
-      this.#normalizeTransaction(address, tx),
+    const normalizedTransactions = await Promise.all(
+      responseTransactions.map((tx) => this.#normalizeTransaction(address, tx)),
     );
 
     log('Normalized transactions', normalizedTransactions);
@@ -188,10 +189,10 @@ export class AccountsApiRemoteTransactionSource
     return filteredTransactions;
   }
 
-  #normalizeTransaction(
+  async #normalizeTransaction(
     address: Hex,
     responseTransaction: GetAccountTransactionsResponse['data'][0],
-  ): TransactionMeta {
+  ): Promise<TransactionMeta> {
     const blockNumber = String(responseTransaction.blockNumber);
     const chainId = `0x${responseTransaction.chainId.toString(16)}` as Hex;
     const { hash } = responseTransaction;
@@ -202,6 +203,7 @@ export class AccountsApiRemoteTransactionSource
     const gasPrice = BNToHex(new BN(responseTransaction.gasPrice));
     const gasUsed = BNToHex(new BN(responseTransaction.gasUsed));
     const nonce = BNToHex(new BN(responseTransaction.nonce));
+    const data = responseTransaction.methodId;
     const type = TransactionType.incoming;
     const verifiedOnBlockchain = false;
 
@@ -211,40 +213,52 @@ export class AccountsApiRemoteTransactionSource
 
     const valueTransfer = responseTransaction.valueTransfers.find(
       (vt) =>
-        vt.to.toLowerCase() === address.toLowerCase() && vt.contractAddress,
+        (vt.to.toLowerCase() === address.toLowerCase() ||
+          vt.from.toLowerCase() === address.toLowerCase()) &&
+        vt.contractAddress,
     );
 
-    const isTransfer = Boolean(valueTransfer);
+    const isIncomingTokenTransfer =
+      valueTransfer?.to.toLowerCase() === address.toLowerCase() &&
+      from.toLowerCase() !== address.toLowerCase();
+
+    const isOutgoing = from.toLowerCase() === address.toLowerCase();
+    const amount = valueTransfer?.amount;
     const contractAddress = valueTransfer?.contractAddress as string;
     const decimals = valueTransfer?.decimal as number;
     const symbol = valueTransfer?.symbol as string;
 
     const value = BNToHex(
-      new BN(valueTransfer?.amount ?? responseTransaction.value),
+      new BN(
+        isIncomingTokenTransfer
+          ? (valueTransfer?.amount ?? responseTransaction.value)
+          : responseTransaction.value,
+      ),
     );
 
-    const to = valueTransfer ? address : responseTransaction.to;
+    const to = isIncomingTokenTransfer ? address : responseTransaction.to;
 
     const error =
       status === TransactionStatus.failed
         ? new Error('Transaction failed')
         : (undefined as unknown as TransactionError);
 
-    const transferInformation = isTransfer
+    const transferInformation = valueTransfer
       ? {
+          amount,
           contractAddress,
           decimals,
           symbol,
         }
       : undefined;
 
-    return {
+    const meta: TransactionMeta = {
       blockNumber,
       chainId,
       error,
       hash,
       id,
-      isTransfer,
+      isTransfer: isIncomingTokenTransfer,
       // Populated by TransactionController when added to state
       networkClientId: '',
       status,
@@ -253,6 +267,7 @@ export class AccountsApiRemoteTransactionSource
       transferInformation,
       txParams: {
         chainId,
+        data,
         from,
         gas,
         gasPrice,
@@ -264,6 +279,12 @@ export class AccountsApiRemoteTransactionSource
       type,
       verifiedOnBlockchain,
     };
+
+    if (isOutgoing) {
+      meta.type = (await determineTransactionType(meta.txParams)).type;
+    }
+
+    return meta;
   }
 
   #updateCache({

packages/transaction-controller/src/types.ts

@@ -228,7 +228,7 @@ export type TransactionMeta = {
   isExternalSign?: boolean;
 
   /**
-   * Whether the transaction is a transfer.
+   * Whether the transaction is an incoming token transfer.
    */
   isTransfer?: boolean;
 
@@ -442,6 +442,7 @@ export type TransactionMeta = {
    * Additional transfer information.
    */
   transferInformation?: {
+    amount?: string;
     contractAddress: string;
     decimals: number;
     symbol: string;

packages/transaction-controller/src/utils/transaction-type.test.ts

@@ -277,4 +277,17 @@ describe('determineTransactionType', () => {
       getCodeResponse: '0xa',
     });
   });
+
+  it('returns contractInteraction if data and no eth query provided', async () => {
+    const result = await determineTransactionType({
+      ...txParams,
+      value: '0x5af3107a4000',
+      data: '0x095ea7b30000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C9700000000000000000000000000000000000000000000000000000000000000005',
+    });
+
+    expect(result).toMatchObject({
+      type: TransactionType.contractInteraction,
+      getCodeResponse: undefined,
+    });
+  });
 });

packages/transaction-controller/src/utils/transaction-type.ts

@@ -1,4 +1,3 @@
-import type { TransactionDescription } from '@ethersproject/abi';
 import { Interface } from '@ethersproject/abi';
 import { query } from '@metamask/controller-utils';
 import type EthQuery from '@metamask/eth-query';
@@ -31,16 +30,23 @@ const USDCInterface = new Interface(abiFiatTokenV2);
  */
 export async function determineTransactionType(
   txParams: TransactionParams,
-  ethQuery: EthQuery,
+  ethQuery?: EthQuery,
 ): Promise<InferTransactionTypeResult> {
   const { data, to } = txParams;
 
   if (data && !to) {
     return { type: TransactionType.deployContract, getCodeResponse: undefined };
   }
 
-  const { contractCode: getCodeResponse, isContractAddress } =
-    await readAddressAsContract(ethQuery, to);
+  let getCodeResponse;
+  let isContractAddress = Boolean(data?.length);
+
+  if (ethQuery) {
+    const response = await readAddressAsContract(ethQuery, to);
+
+    getCodeResponse = response.contractCode;
+    isContractAddress = response.isContractAddress;
+  }
 
   if (!isContractAddress) {
     return { type: TransactionType.simpleSend, getCodeResponse };
@@ -57,7 +63,7 @@ export async function determineTransactionType(
     return contractInteractionResult;
   }
 
-  const name = parseStandardTokenTransactionData(data)?.name;
+  const name = getMethodName(data);
 
   if (!name) {
     return contractInteractionResult;
@@ -89,35 +95,24 @@ export async function determineTransactionType(
  * @param data - Encoded transaction data.
  * @returns A representation of an ethereum contract call.
  */
-function parseStandardTokenTransactionData(
-  data?: string,
-): TransactionDescription | undefined {
-  if (!data) {
+function getMethodName(data?: string): string | undefined {
+  if (!data || data.length < 10) {
     return undefined;
   }
 
-  try {
-    return ERC20Interface.parseTransaction({ data });
-  } catch {
-    // ignore and next try to parse with erc721 ABI
-  }
-
-  try {
-    return ERC721Interface.parseTransaction({ data });
-  } catch {
-    // ignore and next try to parse with erc1155 ABI
-  }
-
-  try {
-    return ERC1155Interface.parseTransaction({ data });
-  } catch {
-    // ignore and return undefined
-  }
-
-  try {
-    return USDCInterface.parseTransaction({ data });
-  } catch {
-    // ignore and return undefined
+  const fourByte = data.substring(0, 10);
+
+  for (const interfaceInstance of [
+    ERC20Interface,
+    ERC721Interface,
+    ERC1155Interface,
+    USDCInterface,
+  ]) {
+    try {
+      return interfaceInstance.getFunction(fourByte).name;
+    } catch {
+      // Intentionally empty
+    }
   }
 
   return undefined;