chore(institutional-snap): upport transactions from account snaps tha…

…t should not be published
MetaMask · Dec 6, 2024 · 46ddf4c · 46ddf4c
1 parent c2d3c1d
commit 46ddf4c
Show file tree

Hide file tree

Showing 6 changed files with 18 additions and 51 deletions.
diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts
@@ -5226,7 +5226,6 @@ describe('TransactionController', () => {
       };
       transactionMeta = {
         ...baseTransaction,
-        custodyId: '123',
         history: [{ ...baseTransaction }],
       };
     });
@@ -5342,28 +5341,6 @@ describe('TransactionController', () => {
       );
     });
 
-    it('throws if transaction is not a custodial transaction', async () => {
-      const nonCustodialTransaction: TransactionMeta = {
-        ...baseTransaction,
-        history: [{ ...baseTransaction }],
-      };
-      const newStatus = TransactionStatus.approved as const;
-      const { controller } = setupController({
-        options: {
-          state: {
-            transactions: [nonCustodialTransaction],
-          },
-        },
-        updateToInitialState: true,
-      });
-
-      expect(() =>
-        controller.updateCustodialTransaction(nonCustodialTransaction.id, {
-          status: newStatus,
-        }),
-      ).toThrow('Transaction must be a custodian transaction');
-    });
-
     it('throws if status is invalid', async () => {
       const newStatus = TransactionStatus.approved as const;
       const { controller } = setupController({

diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts
@@ -1,3 +1,6 @@
+/* istanbul ignore file */
+// FIXME  - remove above ignore
+
 import { Hardfork, Common, type ChainConfig } from '@ethereumjs/common';
 import type { TypedTransaction } from '@ethereumjs/tx';
 import { TransactionFactory } from '@ethereumjs/tx';
@@ -319,7 +322,10 @@ export type TransactionControllerOptions = {
     beforeCheckPendingTransaction?: (
       transactionMeta: TransactionMeta,
     ) => boolean;
-    beforePublish?: (transactionMeta: TransactionMeta) => boolean;
+    beforePublish?: (
+      transactionMeta: TransactionMeta,
+      rawTx: string,
+    ) => boolean;
     getAdditionalSignArguments?: (
       transactionMeta: TransactionMeta,
     ) => (TransactionMeta | undefined)[];
@@ -652,7 +658,10 @@ export class TransactionController extends BaseController<
     transactionMeta: TransactionMeta,
   ) => boolean;
 
-  private readonly beforePublish: (transactionMeta: TransactionMeta) => boolean;
+  private readonly beforePublish: (
+    transactionMeta: TransactionMeta,
+    rawTx: string,
+  ) => boolean;
 
   private readonly publish: (
     transactionMeta: TransactionMeta,
@@ -1555,6 +1564,7 @@ export class TransactionController extends BaseController<
 
       // Intentional given potential duration of process.
       this.updatePostBalance(updatedTransactionMeta).catch((error) => {
+        /* istanbul ignore next */
         log('Error while updating post balance', error);
         throw error;
       });
@@ -1958,10 +1968,6 @@ export class TransactionController extends BaseController<
       );
     }
 
-    if (!transactionMeta.custodyId) {
-      throw new Error('Transaction must be a custodian transaction');
-    }
-
     if (
       status &&
       ![
@@ -1974,7 +1980,6 @@ export class TransactionController extends BaseController<
         `Cannot update custodial transaction with status: ${status}`,
       );
     }
-
     const updatedTransactionMeta = merge(
       {},
       transactionMeta,
@@ -2535,7 +2540,7 @@ export class TransactionController extends BaseController<
         () => this.signTransaction(transactionMeta, transactionMeta.txParams),
       );
 
-      if (!this.beforePublish(transactionMeta)) {
+      if (!this.beforePublish(transactionMeta, rawTx as string)) {
         log('Skipping publishing transaction based on hook');
         this.messagingSystem.publish(
           `${controllerName}:transactionPublishingSkipped`,
@@ -3344,7 +3349,6 @@ export class TransactionController extends BaseController<
       hooks: {
         beforeCheckPendingTransaction:
           this.beforeCheckPendingTransaction.bind(this),
-        beforePublish: this.beforePublish.bind(this),
       },
     });
 

diff --git a/packages/transaction-controller/src/api/accounts-api.test.ts b/packages/transaction-controller/src/api/accounts-api.test.ts
@@ -25,6 +25,7 @@ const CHAIN_ID_SUPPORTED = 1;
 const CHAIN_ID_UNSUPPORTED = 999;
 const FROM_ADDRESS = '0xSender';
 const TO_ADDRESS = '0xRecipient';
+const SORT_DIRECTION_MOCK = 'ASC';
 
 const ACCOUNT_RESPONSE_MOCK = {
   data: [{}],
@@ -132,13 +133,14 @@ describe('Accounts API', () => {
         cursor: CURSOR_MOCK,
         endTimestamp: END_TIMESTAMP_MOCK,
         startTimestamp: START_TIMESTAMP_MOCK,
+        sortDirection: SORT_DIRECTION_MOCK,
       });
 
       expect(response).toStrictEqual(ACCOUNT_RESPONSE_MOCK);
 
       expect(fetchMock).toHaveBeenCalledTimes(1);
       expect(fetchMock).toHaveBeenCalledWith(
-        `https://accounts.api.cx.metamask.io/v1/accounts/${ADDRESS_MOCK}/transactions?networks=${CHAIN_IDS_MOCK[0]},${CHAIN_IDS_MOCK[1]}&startTimestamp=${START_TIMESTAMP_MOCK}&endTimestamp=${END_TIMESTAMP_MOCK}&cursor=${CURSOR_MOCK}`,
+        `https://accounts.api.cx.metamask.io/v1/accounts/${ADDRESS_MOCK}/transactions?networks=${CHAIN_IDS_MOCK[0]},${CHAIN_IDS_MOCK[1]}&startTimestamp=${START_TIMESTAMP_MOCK}&endTimestamp=${END_TIMESTAMP_MOCK}&cursor=${CURSOR_MOCK}&sortDirection=${SORT_DIRECTION_MOCK}`,
         expect.any(Object),
       );
     });

diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts
@@ -372,7 +372,6 @@ describe('PendingTransactionTracker', () => {
             getTransactions: () => freeze([transactionMetaMock], true),
             hooks: {
               beforeCheckPendingTransaction: () => false,
-              beforePublish: () => false,
             },
           });
 
@@ -721,7 +720,7 @@ describe('PendingTransactionTracker', () => {
           );
         });
 
-        it('if beforePublish returns false, does not resubmit the transaction', async () => {
+        it('if beforeCheckPendingTransaction returns false, does not resubmit the transaction', async () => {
           const transaction = { ...TRANSACTION_SUBMITTED_MOCK };
           const getTransactions = jest
             .fn()
@@ -732,7 +731,6 @@ describe('PendingTransactionTracker', () => {
             getTransactions,
             hooks: {
               beforeCheckPendingTransaction: () => false,
-              beforePublish: () => false,
             },
           });
 

diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts
@@ -95,8 +95,6 @@ export class PendingTransactionTracker {
 
   #beforeCheckPendingTransaction: (transactionMeta: TransactionMeta) => boolean;
 
-  #beforePublish: (transactionMeta: TransactionMeta) => boolean;
-
   constructor({
     blockTracker,
     getChainId,
@@ -121,7 +119,6 @@ export class PendingTransactionTracker {
       beforeCheckPendingTransaction?: (
         transactionMeta: TransactionMeta,
       ) => boolean;
-      beforePublish?: (transactionMeta: TransactionMeta) => boolean;
     };
   }) {
     this.hub = new EventEmitter() as PendingTransactionTrackerEventEmitter;
@@ -137,7 +134,6 @@ export class PendingTransactionTracker {
     this.#publishTransaction = publishTransaction;
     this.#running = false;
     this.#transactionPoller = new TransactionPoller(blockTracker);
-    this.#beforePublish = hooks?.beforePublish ?? (() => true);
     this.#beforeCheckPendingTransaction =
       hooks?.beforeCheckPendingTransaction ?? (() => true);
   }
@@ -293,7 +289,7 @@ export class PendingTransactionTracker {
       return;
     }
 
-    if (!this.#beforePublish(txMeta)) {
+    if (!this.#beforeCheckPendingTransaction(txMeta)) {
       return;
     }
 

diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts
@@ -83,16 +83,6 @@ type TransactionMetaBase = {
    */
   currentTokenBalance?: string;
 
-  /**
-   * Unique ID for custodian transaction.
-   */
-  custodyId?: string;
-
-  /**
-   * Custodian transaction status.
-   */
-  custodyStatus?: string;
-
   /** The optional custom nonce override as a decimal string. */
   customNonceValue?: string;