fix: update transaction controllers to use selected account (#4244)

This pr updates the transaction controller to use account id from the
InternalAccount instead of an address

Related to https://github.com/MetaMask/accounts-planning/issues/381

- **BREAKING**: `getSelectedAddress` is replaced with
`getSelectedAccount` in the `TransactionController`
- **BREAKING**: `getCurrentAccount` returns an `InternalAccount` instead
of a `string` in the `IncomingTransactionHelper`

- [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 highlighted breaking changes using the "BREAKING" category
above as appropriate

Fixes https://github.com/MetaMask/accounts-planning/issues/381

---------

Co-authored-by: Charly Chevalier <[email protected]>

Author: 2 people

packages/transaction-controller/package.json

@@ -66,9 +66,11 @@
   },
   "devDependencies": {
     "@babel/runtime": "^7.23.9",
+    "@metamask/accounts-controller": "^16.0.0",
     "@metamask/auto-changelog": "^3.4.4",
     "@metamask/eth-json-rpc-provider": "^4.0.0",
     "@metamask/ethjs-provider-http": "^0.3.0",
+    "@metamask/keyring-api": "^6.4.0",
     "@types/bn.js": "^5.1.5",
     "@types/jest": "^27.4.1",
     "@types/node": "^16.18.54",
@@ -84,6 +86,7 @@
   },
   "peerDependencies": {
     "@babel/runtime": "^7.23.9",
+    "@metamask/accounts-controller": "^16.0.0",
     "@metamask/approval-controller": "^6.0.2",
     "@metamask/gas-fee-controller": "^16.0.0",
     "@metamask/network-controller": "^18.1.3"

packages/transaction-controller/src/TransactionController.test.ts

@@ -17,6 +17,8 @@ import {
 import type { SafeEventEmitterProvider } from '@metamask/eth-json-rpc-provider';
 import EthQuery from '@metamask/eth-query';
 import HttpProvider from '@metamask/ethjs-provider-http';
+import type { InternalAccount } from '@metamask/keyring-api';
+import { EthAccountType } from '@metamask/keyring-api';
 import type {
   BlockTracker,
   NetworkController,
@@ -439,6 +441,20 @@ const MOCK_CUSTOM_NETWORK: MockNetwork = {
 };
 
 const ACCOUNT_MOCK = '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207';
+const INTERNAL_ACCOUNT_MOCK = {
+  id: '58def058-d35f-49a1-a7ab-e2580565f6f5',
+  address: ACCOUNT_MOCK,
+  type: EthAccountType.Eoa,
+  options: {},
+  methods: [],
+  metadata: {
+    name: 'Account 1',
+    keyring: { type: 'HD Key Tree' },
+    importTime: 1631619180000,
+    lastSelected: 1631619180000,
+  },
+};
+
 const ACCOUNT_2_MOCK = '0x08f137f335ea1b8f193b8f6ea92561a60d23a211';
 const NONCE_MOCK = 12;
 const ACTION_ID_MOCK = '123456';
@@ -551,12 +567,14 @@ describe('TransactionController', () => {
    * messenger.
    * @param args.messengerOptions.addTransactionApprovalRequest - Options to mock
    * the `ApprovalController:addRequest` action call for transactions.
+   * @param args.selectedAccount - The selected account to use with the controller.
    * @returns The new TransactionController instance.
    */
   function setupController({
     options: givenOptions = {},
     network = MOCK_NETWORK,
     messengerOptions = {},
+    selectedAccount = INTERNAL_ACCOUNT_MOCK,
   }: {
     options?: Partial<ConstructorParameters<typeof TransactionController>[0]>;
     network?: MockNetwork;
@@ -565,6 +583,7 @@ describe('TransactionController', () => {
         typeof mockAddTransactionApprovalRequest
       >[1];
     };
+    selectedAccount?: InternalAccount;
   } = {}) {
     const unrestrictedMessenger: UnrestrictedControllerMessenger =
       new ControllerMessenger();
@@ -587,7 +606,6 @@ describe('TransactionController', () => {
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       getNetworkClientRegistry: () => ({} as any),
       getPermittedAccounts: async () => [ACCOUNT_MOCK],
-      getSelectedAddress: () => ACCOUNT_MOCK,
       isMultichainEnabled: false,
       hooks: {},
       onNetworkStateChange: network.subscribe,
@@ -605,10 +623,17 @@ describe('TransactionController', () => {
           'ApprovalController:addRequest',
           'NetworkController:getNetworkClientById',
           'NetworkController:findNetworkClientIdByChainId',
+          'AccountsController:getSelectedAccount',
         ],
         allowedEvents: [],
       });
 
+    const mockGetSelectedAccount = jest.fn().mockReturnValue(selectedAccount);
+    unrestrictedMessenger.registerActionHandler(
+      'AccountsController:getSelectedAccount',
+      mockGetSelectedAccount,
+    );
+
     const controller = new TransactionController({
       ...otherOptions,
       messenger: restrictedMessenger,
@@ -618,6 +643,7 @@ describe('TransactionController', () => {
       controller,
       messenger: unrestrictedMessenger,
       mockTransactionApprovalRequest,
+      mockGetSelectedAccount,
     };
   }
 

packages/transaction-controller/src/TransactionController.ts

@@ -2,6 +2,7 @@ import { Hardfork, Common, type ChainConfig } from '@ethereumjs/common';
 import type { TypedTransaction } from '@ethereumjs/tx';
 import { TransactionFactory } from '@ethereumjs/tx';
 import { bufferToHex } from '@ethereumjs/util';
+import type { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller';
 import type {
   AcceptResultCallbacks,
   AddApprovalRequest,
@@ -297,7 +298,6 @@ export type TransactionControllerOptions = {
   getNetworkState: () => NetworkState;
   getPermittedAccounts: (origin?: string) => Promise<string[]>;
   getSavedGasFees?: (chainId: Hex) => SavedGasFees | undefined;
-  getSelectedAddress: () => string;
   incomingTransactions?: IncomingTransactionOptions;
   isMultichainEnabled: boolean;
   isSimulationEnabled?: () => boolean;
@@ -344,7 +344,8 @@ const controllerName = 'TransactionController';
 export type AllowedActions =
   | AddApprovalRequest
   | NetworkControllerFindNetworkClientIdByChainIdAction
-  | NetworkControllerGetNetworkClientByIdAction;
+  | NetworkControllerGetNetworkClientByIdAction
+  | AccountsControllerGetSelectedAccountAction;
 
 /**
  * The external events available to the {@link TransactionController}.
@@ -614,8 +615,6 @@ export class TransactionController extends BaseController<
 
   private readonly getPermittedAccounts: (origin?: string) => Promise<string[]>;
 
-  private readonly getSelectedAddress: () => string;
-
   private readonly getExternalPendingTransactions: (
     address: string,
     chainId?: string,
@@ -733,7 +732,6 @@ export class TransactionController extends BaseController<
    * @param options.getNetworkState - Gets the state of the network controller.
    * @param options.getPermittedAccounts - Get accounts that a given origin has permissions for.
    * @param options.getSavedGasFees - Gets the saved gas fee config.
-   * @param options.getSelectedAddress - Gets the address of the currently selected account.
    * @param options.incomingTransactions - Configuration options for incoming transaction support.
    * @param options.isMultichainEnabled - Enable multichain support.
    * @param options.isSimulationEnabled - Whether new transactions will be automatically simulated.
@@ -761,7 +759,6 @@ export class TransactionController extends BaseController<
     getNetworkState,
     getPermittedAccounts,
     getSavedGasFees,
-    getSelectedAddress,
     incomingTransactions = {},
     isMultichainEnabled = false,
     isSimulationEnabled,
@@ -802,7 +799,6 @@ export class TransactionController extends BaseController<
     this.getGasFeeEstimates =
       getGasFeeEstimates || (() => Promise.resolve({} as GasFeeState));
     this.getPermittedAccounts = getPermittedAccounts;
-    this.getSelectedAddress = getSelectedAddress;
     this.getExternalPendingTransactions =
       getExternalPendingTransactions ?? (() => []);
     this.securityProviderRequest = securityProviderRequest;
@@ -1035,7 +1031,7 @@ export class TransactionController extends BaseController<
     if (origin) {
       await validateTransactionOrigin(
         await this.getPermittedAccounts(origin),
-        this.getSelectedAddress(),
+        this.#getSelectedAccount().address,
         txParams.from,
         origin,
       );
@@ -3430,7 +3426,7 @@ export class TransactionController extends BaseController<
   }): IncomingTransactionHelper {
     const incomingTransactionHelper = new IncomingTransactionHelper({
       blockTracker,
-      getCurrentAccount: this.getSelectedAddress,
+      getCurrentAccount: () => this.#getSelectedAccount(),
       getLastFetchedBlockNumbers: () => this.state.lastFetchedBlockNumbers,
       getChainId: chainId ? () => chainId : this.getChainId.bind(this),
       isEnabled: this.#incomingTransactionOptions.isEnabled,
@@ -3843,4 +3839,8 @@ export class TransactionController extends BaseController<
       ).configuration.type === NetworkClientType.Custom
     );
   }
+
+  #getSelectedAccount() {
+    return this.messagingSystem.call('AccountsController:getSelectedAccount');
+  }
 }