fix: spam filter was not triggered on failed transactions

Author: ulissesferreira

packages/snap/snap.manifest.json

@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snap-solana-wallet.git"
   },
   "source": {
-    "shasum": "15lDW5ySUYnyrQuOe18pdCylumtDe83o1t3rLuMLWWw=",
+    "shasum": "Cto8uBIgowP+zig7Ta1iYWJmzVrm2sPCOaQxpUajukc=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

packages/snap/src/core/services/transactions/utils/mapRpcTransaction.test.ts

@@ -206,29 +206,9 @@ describe('mapRpcTransaction', () => {
       timestamp: 1736500242,
       chain: Network.Mainnet,
       status: 'failed',
-      type: 'send',
-      from: [
-        {
-          address: 'BLw3RweJmfbTapJRgnPRvd962YDjFYAnVGd1p5hmZ5tP',
-          asset: {
-            fungible: true,
-            type: Networks[Network.Mainnet].nativeToken.caip19Id,
-            unit: Networks[Network.Mainnet].nativeToken.symbol,
-            amount: '0.1',
-          },
-        },
-      ],
-      to: [
-        {
-          address: 'FDUGdV6bjhvw5gbirXCvqbTSWK9999kcrZcrHoCQzXJK',
-          asset: {
-            fungible: true,
-            type: Networks[Network.Mainnet].nativeToken.caip19Id,
-            unit: Networks[Network.Mainnet].nativeToken.symbol,
-            amount: '0.1',
-          },
-        },
-      ],
+      type: 'unknown',
+      from: [],
+      to: [],
       fees: [
         {
           type: 'base',
@@ -1115,11 +1095,10 @@ describe('mapRpcTransaction', () => {
     expect(result).toBeNull();
   });
 
-  // TODO: This should be enabled when we support the filtering of spam transactions
-  it.skip('returns null if the transaction is a spam transaction - #2', () => {
+  it('returns null if the transaction is a spam transaction - #2', () => {
     const result = mapRpcTransaction({
       scope: Network.Mainnet,
-      address: asAddress('DAXnAudMEqiD1sS1rFn4ds3pdybRYJd9J58PqCncVVqS'),
+      address: asAddress('FQT9SSwEZ6UUQxsmTzgt5JzjrS4M5zm13M1QiYF8TEo6'),
       transactionData: EXPECTED_SPAM_TRANSACTION_DATA_2,
     });
 

packages/snap/src/core/services/transactions/utils/mapRpcTransaction.ts

@@ -9,6 +9,7 @@ import { BigNumber } from 'bignumber.js';
 import { KnownCaip19Id, type Network } from '../../../constants/solana';
 import type { SolanaTransaction } from '../../../types/solana';
 import { parseTransactionNativeTransfers } from './parseTransactionNativeTransfers';
+import { parseTransactionNativeTransfersV2 } from './parseTransactionNativeTransfersV2';
 import { parseTransactionSplTransfers } from './parseTransactionSplTransfers';
 
 /**
@@ -40,14 +41,37 @@ export function mapRpcTransaction({
 
   const id = firstSignature as string;
   const timestamp = Number(transactionData.blockTime);
+  const status = evaluateTransactionStatus(transactionData);
 
-  const nativeTransfers = parseTransactionNativeTransfers({
-    scope,
-    transactionData,
-  });
+  let fees: Transaction['fees'] = [];
+  let nativeFrom: Transaction['from'] = [];
+  let nativeTo: Transaction['to'] = [];
 
-  let { fees } = nativeTransfers;
-  const { from: nativeFrom, to: nativeTo } = nativeTransfers;
+  if (status === TransactionStatus.Failed) {
+    /**
+     * If a transaction fails we don't really have access to meaningful `preBalances`
+     * and `postBalances` values, since nothing really happened. To extract information
+     * from it we have created this second version of the native transfers parser which
+     * reads instructions directly, instead of relying on balance differences.
+     */
+    const nativeTransfers = parseTransactionNativeTransfersV2({
+      scope,
+      transactionData,
+    });
+
+    fees = nativeTransfers.fees;
+    nativeFrom = nativeTransfers.from;
+    nativeTo = nativeTransfers.to;
+  } else {
+    const nativeTransfers = parseTransactionNativeTransfers({
+      scope,
+      transactionData,
+    });
+
+    fees = nativeTransfers.fees;
+    nativeFrom = nativeTransfers.from;
+    nativeTo = nativeTransfers.to;
+  }
 
   const { from: splFrom, to: splTo } = parseTransactionSplTransfers({
     scope,
@@ -59,6 +83,7 @@ export function mapRpcTransaction({
 
   const type = evaluateTransactionType({
     address,
+    status,
     from,
     to,
   });
@@ -84,12 +109,6 @@ export function mapRpcTransaction({
     fees = [];
   }
 
-  const status =
-    transactionData.meta?.err ||
-    (transactionData.meta?.status && 'Err' in transactionData.meta.status)
-      ? TransactionStatus.Failed
-      : TransactionStatus.Confirmed;
-
   const isLegitimate = evaluateTransactionLegitimacy({
     address,
     from,
@@ -102,6 +121,17 @@ export function mapRpcTransaction({
     return null;
   }
 
+  /**
+   * We cannot do this filtering earlier because we need to use the incomplete
+   * mapped `from` and `to` arrays to determine the transaction's legitimacy.
+   * For failed transactions, we need to first check if they are not spam before
+   * finally clearing out what we had mapped to `from` and `to`.
+   */
+  if (status === TransactionStatus.Failed) {
+    from = [];
+    to = [];
+  }
+
   return {
     id,
     account: address,
@@ -121,25 +151,50 @@ export function mapRpcTransaction({
   };
 }
 
+/**
+ * Evaluates the status of a transaction based on the transaction data.
+ * @param transactionData - The transaction data.
+ * @returns The status of the transaction.
+ */
+function evaluateTransactionStatus(
+  transactionData: SolanaTransaction,
+): TransactionStatus {
+  const isError =
+    transactionData.meta?.err ||
+    (transactionData.meta?.status && 'Err' in transactionData.meta.status);
+
+  const status = isError
+    ? TransactionStatus.Failed
+    : TransactionStatus.Confirmed;
+
+  return status;
+}
+
 /**
  * Evaluates the type of transaction based on the address and the from and to items.
  * @param params - The options object.
  * @param params.address - The address of the user.
  * @param params.from - The from items.
  * @param params.to - The to items.
+ * @param params.status - The status of the transaction.
  * @returns The type of transaction.
  */
 function evaluateTransactionType({
   address,
+  status,
   from,
   to,
 }: {
   address: Address;
+  status: TransactionStatus;
   from: Transaction['from'];
   to: Transaction['to'];
 }): TransactionType {
-  if (from.length === 0 || to.length === 0) {
-    // if we are unable to determine the type of transaction, we should set it to unknown
+  if (
+    from.length === 0 ||
+    to.length === 0 ||
+    status === TransactionStatus.Failed
+  ) {
     return TransactionType.Unknown;
   }
 
@@ -203,13 +258,13 @@ function passesSOLAmountThresholdCheck({
   to,
 }: {
   address: Address;
-  to: Transaction['to'];
+  to: Transaction['from'];
 }): boolean {
   const { hasReceivedSOL, receivedSOLAmount } = to.reduce(
     (acc, toItem) => {
       if (
         toItem.address === address &&
-        toItem.asset?.fungible && // Use optional chaining here
+        toItem.asset?.fungible &&
         (toItem.asset.type === KnownCaip19Id.SolMainnet ||
           toItem.asset.type === KnownCaip19Id.SolDevnet)
       ) {

packages/snap/src/core/services/transactions/utils/parseTransactionNativeTransfers.ts

@@ -13,7 +13,7 @@ import { lamportsToSol } from '../../../utils/conversion';
 import { parseTransactionFees } from './parseTransactionFees';
 
 /**
- * Parses native SOL token transfers from a transaction.
+ * Parses native SOL token transfers from a transaction using its balance changes.
  * @param options0 - The options object.
  * @param options0.scope - The network scope (e.g., Mainnet, Devnet).
  * @param options0.transactionData - The raw transaction data containing balance changes.