Tokens controller approve reject refactor (#1261)

* stops calling approve and reject from controller
MetaMask · Oct 11, 2023 · b49edf4 · b49edf4
1 parent ae9b12d
commit b49edf4
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 92 deletions.
diff --git a/packages/assets-controllers/src/TokensController.test.ts b/packages/assets-controllers/src/TokensController.test.ts
@@ -1,10 +1,6 @@
 import * as sinon from 'sinon';
 import nock from 'nock';
-import {
-  AcceptRequest as AcceptApprovalRequest,
-  AddApprovalRequest,
-  RejectRequest as RejectApprovalRequest,
-} from '@metamask/approval-controller';
+import { AddApprovalRequest } from '@metamask/approval-controller';
 import { ControllerMessenger } from '@metamask/base-controller';
 import contractMaps from '@metamask/contract-metadata';
 import { PreferencesController } from '@metamask/preferences-controller';
@@ -46,10 +42,7 @@ const GOERLI = { chainId: '5', type: NetworkType.goerli };
 
 const controllerName = 'TokensController' as const;
 
-type ApprovalActions =
-  | AddApprovalRequest
-  | AcceptApprovalRequest
-  | RejectApprovalRequest;
+type ApprovalActions = AddApprovalRequest;
 
 describe('TokensController', () => {
   let tokensController: TokensController;
@@ -60,11 +53,7 @@ describe('TokensController', () => {
     never
   >().getRestricted<typeof controllerName, ApprovalActions['type'], never>({
     name: controllerName,
-    allowedActions: [
-      'ApprovalController:addRequest',
-      'ApprovalController:acceptRequest',
-      'ApprovalController:rejectRequest',
-    ],
+    allowedActions: ['ApprovalController:addRequest'],
   }) as TokensControllerMessenger;
 
   let onNetworkStateChangeListener: (state: NetworkState) => void;
@@ -1096,7 +1085,7 @@ describe('TokensController', () => {
             image: 'image',
           },
         ]);
-        expect(callActionSpy).toHaveBeenCalledTimes(2);
+        expect(callActionSpy).toHaveBeenCalledTimes(1);
         expect(callActionSpy).toHaveBeenCalledWith(
           'ApprovalController:addRequest',
           {
@@ -1116,10 +1105,6 @@ describe('TokensController', () => {
           },
           true,
         );
-        expect(callActionSpy).toHaveBeenCalledWith(
-          'ApprovalController:acceptRequest',
-          expect.any(String),
-        );
 
         generateRandomIdStub.restore();
       },
@@ -1152,7 +1137,7 @@ describe('TokensController', () => {
           image: 'image',
         },
       ]);
-      expect(callActionSpy).toHaveBeenCalledTimes(2);
+      expect(callActionSpy).toHaveBeenCalledTimes(1);
       expect(callActionSpy).toHaveBeenCalledWith(
         'ApprovalController:addRequest',
         {
@@ -1172,10 +1157,6 @@ describe('TokensController', () => {
         },
         true,
       );
-      expect(callActionSpy).toHaveBeenCalledWith(
-        'ApprovalController:acceptRequest',
-        expect.any(String),
-      );
 
       generateRandomIdStub.restore();
     });
@@ -1228,7 +1209,7 @@ describe('TokensController', () => {
         await expect(result).rejects.toThrow(
           'User rejected to watch the asset.',
         );
-        expect(callActionSpy).toHaveBeenCalledTimes(2);
+        expect(callActionSpy).toHaveBeenCalledTimes(1);
         expect(callActionSpy).toHaveBeenCalledWith(
           'ApprovalController:addRequest',
           {
@@ -1248,11 +1229,6 @@ describe('TokensController', () => {
           },
           true,
         );
-        expect(callActionSpy).toHaveBeenCalledWith(
-          'ApprovalController:rejectRequest',
-          expect.any(String),
-          expect.any(Error),
-        );
       },
     );
 
@@ -1270,7 +1246,7 @@ describe('TokensController', () => {
       const res = await result;
       expect(tokensController.state.suggestedAssets).toHaveLength(0);
       expect(res).toBe('0xe9f786dfdd9ae4d57e830acb52296837765f0e5b');
-      expect(callActionSpy).toHaveBeenCalledTimes(2);
+      expect(callActionSpy).toHaveBeenCalledTimes(1);
       expect(callActionSpy).toHaveBeenCalledWith(
         'ApprovalController:addRequest',
         {
@@ -1290,10 +1266,6 @@ describe('TokensController', () => {
         },
         true,
       );
-      expect(callActionSpy).toHaveBeenCalledWith(
-        'ApprovalController:acceptRequest',
-        expect.any(String),
-      );
     });
 
     it.each([
@@ -1336,7 +1308,7 @@ describe('TokensController', () => {
         await expect(result).rejects.toThrow(
           'Asset of type ERC721 not supported',
         );
-        expect(callActionSpy).toHaveBeenCalledTimes(2);
+        expect(callActionSpy).toHaveBeenCalledTimes(1);
         expect(callActionSpy).toHaveBeenCalledWith(
           'ApprovalController:addRequest',
           {
@@ -1356,11 +1328,6 @@ describe('TokensController', () => {
           },
           true,
         );
-        expect(callActionSpy).toHaveBeenCalledWith(
-          'ApprovalController:rejectRequest',
-          expect.any(String),
-          expect.any(Error),
-        );
       },
     );
   });

diff --git a/packages/assets-controllers/src/TokensController.ts b/packages/assets-controllers/src/TokensController.ts
@@ -1,9 +1,5 @@
 import { EventEmitter } from 'events';
-import {
-  AcceptRequest as AcceptApprovalRequest,
-  AddApprovalRequest,
-  RejectRequest as RejectApprovalRequest,
-} from '@metamask/approval-controller';
+import { AddApprovalRequest } from '@metamask/approval-controller';
 import contractsMap from '@metamask/contract-metadata';
 import { abiERC721 } from '@metamask/metamask-eth-abis';
 import { v1 as random } from 'uuid';
@@ -71,7 +67,7 @@ export type SuggestedAssetMetaBase = {
   time: number;
   type: string;
   asset: Token;
-  interactingAddress?: string;
+  interactingAddress: string;
 };
 
 /**
@@ -128,10 +124,7 @@ const controllerName = 'TokensController';
 /**
  * The external actions available to the {@link TokensController}.
  */
-type AllowedActions =
-  | AddApprovalRequest
-  | AcceptApprovalRequest
-  | RejectApprovalRequest;
+type AllowedActions = AddApprovalRequest;
 
 /**
  * The messenger of the {@link TokensController}.
@@ -666,12 +659,10 @@ export class TokensController extends BaseController<
   ): Promise<AssetSuggestionResult> {
     const { selectedAddress } = this.config;
 
-    const suggestedAssetMeta: SuggestedAssetMeta & {
-      interactingAddress: string;
-    } = {
+    const suggestedAssetMeta: SuggestedAssetMeta = {
       asset,
       id: this._generateRandomId(),
-      status: SuggestedAssetStatus.pending as SuggestedAssetStatus.pending,
+      status: SuggestedAssetStatus.pending,
       time: Date.now(),
       type,
       interactingAddress: interactingAddress || selectedAddress,
@@ -744,8 +735,6 @@ export class TokensController extends BaseController<
             suggestedAssetMeta?.interactingAddress || selectedAddress,
           );
 
-          this._acceptApproval(suggestedAssetID);
-
           const acceptedSuggestedAssetMeta = {
             ...suggestedAssetMeta,
             status: SuggestedAssetStatus.accepted,
@@ -762,8 +751,6 @@ export class TokensController extends BaseController<
       }
     } catch (error) {
       this.failSuggestedAsset(suggestedAssetMeta, error);
-
-      this._rejectApproval(suggestedAssetID);
     }
 
     const newSuggestedAssets = suggestedAssets.filter(
@@ -799,8 +786,6 @@ export class TokensController extends BaseController<
       ({ id }) => id !== suggestedAssetID,
     );
     this.update({ suggestedAssets: [...newSuggestedAssets] });
-
-    this._rejectApproval(suggestedAssetID);
   }
 
   /**
@@ -901,11 +886,7 @@ export class TokensController extends BaseController<
     this.update({ ignoredTokens: [], allIgnoredTokens: {} });
   }
 
-  _requestApproval(
-    suggestedAssetMeta: SuggestedAssetMeta & {
-      interactingAddress: string;
-    },
-  ) {
+  _requestApproval(suggestedAssetMeta: SuggestedAssetMeta) {
     this.messagingSystem
       .call(
         'ApprovalController:addRequest',
@@ -930,32 +911,6 @@ export class TokensController extends BaseController<
         // Intentionally ignored as promise not currently used
       });
   }
-
-  _acceptApproval(approvalRequestId: string) {
-    try {
-      this.messagingSystem.call(
-        'ApprovalController:acceptRequest',
-        approvalRequestId,
-      );
-    } catch (error) {
-      console.error('Failed to accept token watch approval request', error);
-    }
-  }
-
-  _rejectApproval(approvalRequestId: string) {
-    try {
-      this.messagingSystem.call(
-        'ApprovalController:rejectRequest',
-        approvalRequestId,
-        new Error('Rejected'),
-      );
-    } catch (messageCallError) {
-      console.error(
-        'Failed to reject token watch approval request',
-        messageCallError,
-      );
-    }
-  }
 }
 
 export default TokensController;