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

…t should not be published
MetaMask · Dec 9, 2024 · e58ce4a · e58ce4a
1 parent 4000c4d
commit e58ce4a
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 50 deletions.
diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts
@@ -5256,7 +5256,6 @@ describe('TransactionController', () => {
       };
       transactionMeta = {
         ...baseTransaction,
-        custodyId: '123',
         history: [{ ...baseTransaction }],
       };
     });
@@ -5372,28 +5371,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';
@@ -318,7 +321,10 @@ export type TransactionControllerOptions = {
     beforeCheckPendingTransaction?: (
       transactionMeta: TransactionMeta,
     ) => boolean;
-    beforePublish?: (transactionMeta: TransactionMeta) => boolean;
+    beforePublish?: (
+      transactionMeta: TransactionMeta,
+      rawTx: string,
+    ) => boolean;
     getAdditionalSignArguments?: (
       transactionMeta: TransactionMeta,
     ) => (TransactionMeta | undefined)[];
@@ -647,7 +653,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,
@@ -1519,6 +1528,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;
       });
@@ -1922,10 +1932,6 @@ export class TransactionController extends BaseController<
       );
     }
 
-    if (!transactionMeta.custodyId) {
-      throw new Error('Transaction must be a custodian transaction');
-    }
-
     if (
       status &&
       ![
@@ -1938,7 +1944,6 @@ export class TransactionController extends BaseController<
         `Cannot update custodial transaction with status: ${status}`,
       );
     }
-
     const updatedTransactionMeta = merge(
       {},
       transactionMeta,
@@ -2499,7 +2504,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`,
@@ -3356,7 +3361,6 @@ export class TransactionController extends BaseController<
       hooks: {
         beforeCheckPendingTransaction:
           this.beforeCheckPendingTransaction.bind(this),
-        beforePublish: this.beforePublish.bind(this),
       },
     });
 

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
@@ -93,8 +93,6 @@ export class PendingTransactionTracker {
 
   #beforeCheckPendingTransaction: (transactionMeta: TransactionMeta) => boolean;
 
-  #beforePublish: (transactionMeta: TransactionMeta) => boolean;
-
   constructor({
     blockTracker,
     getChainId,
@@ -119,7 +117,6 @@ export class PendingTransactionTracker {
       beforeCheckPendingTransaction?: (
         transactionMeta: TransactionMeta,
       ) => boolean;
-      beforePublish?: (transactionMeta: TransactionMeta) => boolean;
     };
   }) {
     this.hub = new EventEmitter() as PendingTransactionTrackerEventEmitter;
@@ -135,7 +132,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);
   }
@@ -291,7 +287,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;