From b874e4cc8a5ad73b2f6519b8761f333402fa4cf8 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Mon, 3 Jun 2024 22:36:31 +0800 Subject: [PATCH 01/10] feat: update transaction controller to selectedAccount --- packages/transaction-controller/package.json | 1 + .../src/TransactionController.test.ts | 17 +++- .../src/TransactionController.ts | 17 ++-- .../TransactionControllerIntegration.test.ts | 89 ++++++++++++++++--- .../helpers/IncomingTransactionHelper.test.ts | 19 +++- .../src/helpers/IncomingTransactionHelper.ts | 14 +-- yarn.lock | 3 +- 7 files changed, 131 insertions(+), 29 deletions(-) diff --git a/packages/transaction-controller/package.json b/packages/transaction-controller/package.json index 3bc8e97121..a83eb445be 100644 --- a/packages/transaction-controller/package.json +++ b/packages/transaction-controller/package.json @@ -52,6 +52,7 @@ "@metamask/controller-utils": "^11.0.0", "@metamask/eth-query": "^4.0.0", "@metamask/gas-fee-controller": "^17.0.0", + "@metamask/keyring-api": "6.4.0", "@metamask/metamask-eth-abis": "^3.1.1", "@metamask/network-controller": "^19.0.0", "@metamask/nonce-tracker": "^5.0.0", diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 5cfbdb33fb..5cc0ede74a 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -16,6 +16,7 @@ import { } from '@metamask/controller-utils'; import EthQuery from '@metamask/eth-query'; import HttpProvider from '@metamask/ethjs-provider-http'; +import { EthAccountType } from '@metamask/keyring-api'; import type { BlockTracker, NetworkController, @@ -434,6 +435,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'; @@ -582,7 +597,7 @@ describe('TransactionController', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any getNetworkClientRegistry: () => ({} as any), getPermittedAccounts: async () => [ACCOUNT_MOCK], - getSelectedAddress: () => ACCOUNT_MOCK, + getSelectedAccount: () => INTERNAL_ACCOUNT_MOCK, isMultichainEnabled: false, hooks: {}, onNetworkStateChange: network.subscribe, diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 06010fc314..202f635ed0 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -25,6 +25,7 @@ import type { FetchGasFeeEstimateOptions, GasFeeState, } from '@metamask/gas-fee-controller'; +import type { InternalAccount } from '@metamask/keyring-api'; import type { BlockTracker, NetworkClientId, @@ -42,7 +43,7 @@ import type { Transaction as NonceTrackerTransaction, } from '@metamask/nonce-tracker'; import { errorCodes, rpcErrors, providerErrors } from '@metamask/rpc-errors'; -import type { Hex } from '@metamask/utils'; +import type { CaipChainId, Hex } from '@metamask/utils'; import { add0x } from '@metamask/utils'; import { Mutex } from 'async-mutex'; import { MethodRegistry } from 'eth-method-registry'; @@ -297,7 +298,7 @@ export type TransactionControllerOptions = { getNetworkState: () => NetworkState; getPermittedAccounts: (origin?: string) => Promise; getSavedGasFees?: (chainId: Hex) => SavedGasFees | undefined; - getSelectedAddress: () => string; + getSelectedAccount: () => InternalAccount; incomingTransactions?: IncomingTransactionOptions; isMultichainEnabled: boolean; isSimulationEnabled?: () => boolean; @@ -614,7 +615,7 @@ export class TransactionController extends BaseController< private readonly getPermittedAccounts: (origin?: string) => Promise; - private readonly getSelectedAddress: () => string; + private readonly getSelectedAccount: () => InternalAccount; private readonly getExternalPendingTransactions: ( address: string, @@ -733,7 +734,7 @@ 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.getSelectedAccount - 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 +762,7 @@ export class TransactionController extends BaseController< getNetworkState, getPermittedAccounts, getSavedGasFees, - getSelectedAddress, + getSelectedAccount, incomingTransactions = {}, isMultichainEnabled = false, isSimulationEnabled, @@ -802,7 +803,7 @@ export class TransactionController extends BaseController< this.getGasFeeEstimates = getGasFeeEstimates || (() => Promise.resolve({} as GasFeeState)); this.getPermittedAccounts = getPermittedAccounts; - this.getSelectedAddress = getSelectedAddress; + this.getSelectedAccount = getSelectedAccount; this.getExternalPendingTransactions = getExternalPendingTransactions ?? (() => []); this.securityProviderRequest = securityProviderRequest; @@ -1035,7 +1036,7 @@ export class TransactionController extends BaseController< if (origin) { await validateTransactionOrigin( await this.getPermittedAccounts(origin), - this.getSelectedAddress(), + this.getSelectedAccount().address, txParams.from, origin, ); @@ -3430,7 +3431,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, diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index 979f88c452..91349913c8 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -11,6 +11,8 @@ import { InfuraNetworkType, NetworkType, } from '@metamask/controller-utils'; +import type { InternalAccount } from '@metamask/keyring-api'; +import { EthAccountType, EthMethod } from '@metamask/keyring-api'; import { NetworkController, NetworkClientType, @@ -25,6 +27,7 @@ import assert from 'assert'; import nock from 'nock'; import type { SinonFakeTimers } from 'sinon'; import { useFakeTimers } from 'sinon'; +import { v4 } from 'uuid'; import { advanceTime } from '../../../tests/helpers'; import { mockNetwork } from '../../../tests/mock-network'; @@ -64,7 +67,46 @@ type UnrestrictedControllerMessenger = ControllerMessenger< | TransactionControllerEvents >; +const createMockInternalAccount = ({ + id = v4(), + address = '0x2990079bcdee240329a520d2444386fc119da21a', + name = 'Account 1', + importTime = Date.now(), + lastSelected = Date.now(), +}: { + id?: string; + address?: string; + name?: string; + importTime?: number; + lastSelected?: number; +} = {}): InternalAccount => { + return { + id, + address, + options: {}, + methods: [ + EthMethod.PersonalSign, + EthMethod.Sign, + EthMethod.SignTransaction, + EthMethod.SignTypedDataV1, + EthMethod.SignTypedDataV3, + EthMethod.SignTypedDataV4, + ], + type: EthAccountType.Eoa, + metadata: { + name, + keyring: { type: 'HD Key Tree' }, + importTime, + lastSelected, + }, + } as InternalAccount; +}; + const ACCOUNT_MOCK = '0x6bf137f335ea1b8f193b8f6ea92561a60d23a207'; +const INTERNAL_ACCOUNT_MOCK = createMockInternalAccount({ + address: ACCOUNT_MOCK, +}); + const ACCOUNT_2_MOCK = '0x08f137f335ea1b8f193b8f6ea92561a60d23a211'; const ACCOUNT_3_MOCK = '0xe688b84b23f322a994a53dbf8e15fa82cdb71127'; const infuraProjectId = 'fake-infura-project-id'; @@ -167,7 +209,8 @@ const setupController = async ( getNetworkClientRegistry: networkController.getNetworkClientRegistry.bind(networkController), getPermittedAccounts: async () => [ACCOUNT_MOCK], - getSelectedAddress: () => '0xdeadbeef', + getSelectedAccount: () => + createMockInternalAccount({ address: '0xdeadbeef' }), hooks: {}, isMultichainEnabled: false, messenger, @@ -802,7 +845,7 @@ describe('TransactionController Integration', () => { await setupController({ isMultichainEnabled: true, getPermittedAccounts: async () => [ACCOUNT_MOCK], - getSelectedAddress: () => ACCOUNT_MOCK, + getSelectedAccount: () => INTERNAL_ACCOUNT_MOCK, }); const otherNetworkClientIdOnGoerli = await networkController.upsertNetworkConfiguration( @@ -883,7 +926,7 @@ describe('TransactionController Integration', () => { await setupController({ isMultichainEnabled: true, getPermittedAccounts: async () => [ACCOUNT_MOCK], - getSelectedAddress: () => ACCOUNT_MOCK, + getSelectedAccount: () => INTERNAL_ACCOUNT_MOCK, }); const addTx1 = await transactionController.addTransaction( @@ -1140,10 +1183,13 @@ describe('TransactionController Integration', () => { }); const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; + const selectedAccountMock = createMockInternalAccount({ + address: selectedAddress, + }); const { networkController, transactionController } = await setupController({ - getSelectedAddress: () => selectedAddress, + getSelectedAccount: () => selectedAccountMock, isMultichainEnabled: true, }); @@ -1209,6 +1255,9 @@ describe('TransactionController Integration', () => { it('should start the global incoming transaction helper when no networkClientIds provided', async () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; + const selectedAccountMock = createMockInternalAccount({ + address: selectedAddress, + }); mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( @@ -1226,7 +1275,7 @@ describe('TransactionController Integration', () => { .reply(200, ETHERSCAN_TRANSACTION_RESPONSE_MOCK); const { transactionController } = await setupController({ - getSelectedAddress: () => selectedAddress, + getSelectedAccount: () => selectedAccountMock, }); transactionController.startIncomingTransactionPolling(); @@ -1314,10 +1363,13 @@ describe('TransactionController Integration', () => { }); const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; + const selectedAccountMock = createMockInternalAccount({ + address: selectedAddress, + }); const { networkController, transactionController } = await setupController({ - getSelectedAddress: () => selectedAddress, + getSelectedAccount: () => selectedAccountMock, isMultichainEnabled: true, }); @@ -1410,10 +1462,13 @@ describe('TransactionController Integration', () => { describe('stopIncomingTransactionPolling', () => { it('should not poll for new incoming transactions for the given networkClientId', async () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; + const selectedAccountMock = createMockInternalAccount({ + address: selectedAddress, + }); const { networkController, transactionController } = await setupController({ - getSelectedAddress: () => selectedAddress, + getSelectedAccount: () => selectedAccountMock, }); const networkClients = networkController.getNetworkClientRegistry(); @@ -1454,9 +1509,12 @@ describe('TransactionController Integration', () => { it('should stop the global incoming transaction helper when no networkClientIds provided', async () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; + const selectedAccountMock = createMockInternalAccount({ + address: selectedAddress, + }); const { transactionController } = await setupController({ - getSelectedAddress: () => selectedAddress, + getSelectedAccount: () => selectedAccountMock, }); mockNetwork({ @@ -1490,10 +1548,13 @@ describe('TransactionController Integration', () => { describe('stopAllIncomingTransactionPolling', () => { it('should not poll for incoming transactions on any network client', async () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; + const selectedAccountMock = createMockInternalAccount({ + address: selectedAddress, + }); const { networkController, transactionController } = await setupController({ - getSelectedAddress: () => selectedAddress, + getSelectedAccount: () => selectedAccountMock, }); const networkClients = networkController.getNetworkClientRegistry(); @@ -1534,10 +1595,13 @@ describe('TransactionController Integration', () => { describe('updateIncomingTransactions', () => { it('should add incoming transactions to state with the correct chainId for the given networkClientId without waiting for the next block', async () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; + const selectedAccountMock = createMockInternalAccount({ + address: selectedAddress, + }); const { networkController, transactionController } = await setupController({ - getSelectedAddress: () => selectedAddress, + getSelectedAccount: () => selectedAccountMock, isMultichainEnabled: true, }); @@ -1600,9 +1664,12 @@ describe('TransactionController Integration', () => { it('should update the incoming transactions for the gloablly selected network when no networkClientIds provided', async () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; + const selectedAccountMock = createMockInternalAccount({ + address: selectedAddress, + }); const { transactionController } = await setupController({ - getSelectedAddress: () => selectedAddress, + getSelectedAccount: () => selectedAccountMock, }); mockNetwork({ diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts index 49b39c4eff..6e65f7de1c 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts @@ -32,7 +32,21 @@ const BLOCK_TRACKER_MOCK = { const CONTROLLER_ARGS_MOCK = { blockTracker: BLOCK_TRACKER_MOCK, - getCurrentAccount: () => ADDRESS_MOCK, + getCurrentAccount: () => { + return { + id: '58def058-d35f-49a1-a7ab-e2580565f6f5', + address: ADDRESS_MOCK, + type: 'eip155:eoa' as const, + options: {}, + methods: [], + metadata: { + name: 'Account 1', + keyring: { type: 'HD Key Tree' }, + importTime: 1631619180000, + lastSelected: 1631619180000, + }, + }; + }, getLastFetchedBlockNumbers: () => ({}), getChainId: () => CHAIN_ID_MOCK, remoteTransactionSource: {} as RemoteTransactionSource, @@ -546,7 +560,8 @@ describe('IncomingTransactionHelper', () => { remoteTransactionSource: createRemoteTransactionSourceMock([ TRANSACTION_MOCK_2, ]), - getCurrentAccount: () => undefined as unknown as string, + // @ts-expect-error testing undefined + getCurrentAccount: () => undefined, }); const { blockNumberListener } = await emitBlockTrackerLatestEvent( diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts index c6600b4893..0b0cf6e93b 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts @@ -1,3 +1,4 @@ +import type { InternalAccount } from '@metamask/keyring-api'; import type { BlockTracker } from '@metamask/network-controller'; import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; @@ -35,7 +36,7 @@ export class IncomingTransactionHelper { #blockTracker: BlockTracker; - #getCurrentAccount: () => string; + #getCurrentAccount: () => InternalAccount; #getLastFetchedBlockNumbers: () => Record; @@ -72,7 +73,7 @@ export class IncomingTransactionHelper { updateTransactions, }: { blockTracker: BlockTracker; - getCurrentAccount: () => string; + getCurrentAccount: () => InternalAccount; getLastFetchedBlockNumbers: () => Record; getLocalTransactions?: () => TransactionMeta[]; getChainId: () => Hex; @@ -144,7 +145,7 @@ export class IncomingTransactionHelper { this.#remoteTransactionSource.getLastBlockVariations?.() ?? []; const fromBlock = this.#getFromBlock(latestBlockNumber); - const address = this.#getCurrentAccount(); + const account = this.#getCurrentAccount(); const currentChainId = this.#getChainId(); let remoteTransactions = []; @@ -152,7 +153,7 @@ export class IncomingTransactionHelper { try { remoteTransactions = await this.#remoteTransactionSource.fetchTransactions({ - address, + address: account.address, currentChainId, fromBlock, limit: this.#transactionLimit, @@ -165,7 +166,8 @@ export class IncomingTransactionHelper { } if (!this.#updateTransactions) { remoteTransactions = remoteTransactions.filter( - (tx) => tx.txParams.to?.toLowerCase() === address.toLowerCase(), + (tx) => + tx.txParams.to?.toLowerCase() === account.address.toLowerCase(), ); } @@ -301,7 +303,7 @@ export class IncomingTransactionHelper { #getBlockNumberKey(additionalKeys: string[]): string { const currentChainId = this.#getChainId(); - const currentAccount = this.#getCurrentAccount()?.toLowerCase(); + const currentAccount = this.#getCurrentAccount()?.address.toLowerCase(); return [currentChainId, currentAccount, ...additionalKeys].join('#'); } diff --git a/yarn.lock b/yarn.lock index 52d067820f..d392449a33 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2463,7 +2463,7 @@ __metadata: languageName: node linkType: hard -"@metamask/keyring-api@npm:^6.3.1, @metamask/keyring-api@npm:^6.4.0": +"@metamask/keyring-api@npm:6.4.0, @metamask/keyring-api@npm:^6.3.1, @metamask/keyring-api@npm:^6.4.0": version: 6.4.0 resolution: "@metamask/keyring-api@npm:6.4.0" dependencies: @@ -3149,6 +3149,7 @@ __metadata: "@metamask/eth-query": ^4.0.0 "@metamask/ethjs-provider-http": ^0.3.0 "@metamask/gas-fee-controller": ^17.0.0 + "@metamask/keyring-api": 6.4.0 "@metamask/metamask-eth-abis": ^3.1.1 "@metamask/network-controller": ^19.0.0 "@metamask/nonce-tracker": ^5.0.0 From 598275b8f14c4a06ef81ef0c7ceeb6e7d16e00ad Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Mon, 3 Jun 2024 22:41:14 +0800 Subject: [PATCH 02/10] fix: lint --- packages/transaction-controller/src/TransactionController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 202f635ed0..b15aaba286 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -43,7 +43,7 @@ import type { Transaction as NonceTrackerTransaction, } from '@metamask/nonce-tracker'; import { errorCodes, rpcErrors, providerErrors } from '@metamask/rpc-errors'; -import type { CaipChainId, Hex } from '@metamask/utils'; +import type { Hex } from '@metamask/utils'; import { add0x } from '@metamask/utils'; import { Mutex } from 'async-mutex'; import { MethodRegistry } from 'eth-method-registry'; From 1f0f6988c8d40a6af7cdb81c5dca86c20ff589b5 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Mon, 3 Jun 2024 22:59:04 +0800 Subject: [PATCH 03/10] Update packages/transaction-controller/src/TransactionController.ts Co-authored-by: Charly Chevalier --- packages/transaction-controller/src/TransactionController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index b15aaba286..dee117ba30 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -734,7 +734,7 @@ 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.getSelectedAccount - Gets the address of the currently selected account. + * @param options.getSelectedAccount - Gets 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. From 45a381903d79705aa09efb4a7647aed3939766dd Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Mon, 3 Jun 2024 23:00:56 +0800 Subject: [PATCH 04/10] fix: move toLowerCase computation out of loop --- .../src/helpers/IncomingTransactionHelper.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts index 0b0cf6e93b..e44a2f6b2c 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts @@ -165,9 +165,9 @@ export class IncomingTransactionHelper { return; } if (!this.#updateTransactions) { + const address = account.address.toLowerCase(); remoteTransactions = remoteTransactions.filter( - (tx) => - tx.txParams.to?.toLowerCase() === account.address.toLowerCase(), + (tx) => tx.txParams.to?.toLowerCase() === address, ); } From df896e5a60565b6ed8684fa111444e0cdaf80193 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 6 Jun 2024 14:55:17 +0800 Subject: [PATCH 05/10] refactor: getSeletedAccount to messenger action --- packages/transaction-controller/package.json | 4 +- .../src/TransactionController.test.ts | 11 +- .../src/TransactionController.ts | 17 +-- .../TransactionControllerIntegration.test.ts | 125 +++++++++++------- .../src/helpers/IncomingTransactionHelper.ts | 10 +- .../tsconfig.build.json | 1 + packages/transaction-controller/tsconfig.json | 1 + yarn.lock | 6 +- 8 files changed, 113 insertions(+), 62 deletions(-) diff --git a/packages/transaction-controller/package.json b/packages/transaction-controller/package.json index a83eb445be..8750547d7d 100644 --- a/packages/transaction-controller/package.json +++ b/packages/transaction-controller/package.json @@ -47,12 +47,12 @@ "@ethersproject/abi": "^5.7.0", "@ethersproject/contracts": "^5.7.0", "@ethersproject/providers": "^5.7.0", + "@metamask/accounts-controller": "^16.0.0", "@metamask/approval-controller": "^7.0.0", "@metamask/base-controller": "^6.0.0", "@metamask/controller-utils": "^11.0.0", "@metamask/eth-query": "^4.0.0", "@metamask/gas-fee-controller": "^17.0.0", - "@metamask/keyring-api": "6.4.0", "@metamask/metamask-eth-abis": "^3.1.1", "@metamask/network-controller": "^19.0.0", "@metamask/nonce-tracker": "^5.0.0", @@ -69,6 +69,7 @@ "@babel/runtime": "^7.23.9", "@metamask/auto-changelog": "^3.4.4", "@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 +85,7 @@ }, "peerDependencies": { "@babel/runtime": "^7.23.9", + "@metamask/accounts-controller": "^16.0.0", "@metamask/approval-controller": "^7.0.0", "@metamask/gas-fee-controller": "^17.0.0", "@metamask/network-controller": "^19.0.0" diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 5cc0ede74a..f5273dfdde 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -597,7 +597,6 @@ describe('TransactionController', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any getNetworkClientRegistry: () => ({} as any), getPermittedAccounts: async () => [ACCOUNT_MOCK], - getSelectedAccount: () => INTERNAL_ACCOUNT_MOCK, isMultichainEnabled: false, hooks: {}, onNetworkStateChange: network.subscribe, @@ -615,10 +614,19 @@ describe('TransactionController', () => { 'ApprovalController:addRequest', 'NetworkController:getNetworkClientById', 'NetworkController:findNetworkClientIdByChainId', + 'AccountsController:getSelectedAccount', ], allowedEvents: [], }); + const mockSelectedAccountCall = jest + .fn() + .mockReturnValue(INTERNAL_ACCOUNT_MOCK); + unrestrictedMessenger.registerActionHandler( + 'AccountsController:getSelectedAccount', + mockSelectedAccountCall, + ); + const controller = new TransactionController({ ...otherOptions, messenger: restrictedMessenger, @@ -628,6 +636,7 @@ describe('TransactionController', () => { controller, messenger: unrestrictedMessenger, mockTransactionApprovalRequest, + mockSelectedAccountCall, }; } diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index dee117ba30..1724f38a30 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/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, @@ -25,7 +26,6 @@ import type { FetchGasFeeEstimateOptions, GasFeeState, } from '@metamask/gas-fee-controller'; -import type { InternalAccount } from '@metamask/keyring-api'; import type { BlockTracker, NetworkClientId, @@ -298,7 +298,6 @@ export type TransactionControllerOptions = { getNetworkState: () => NetworkState; getPermittedAccounts: (origin?: string) => Promise; getSavedGasFees?: (chainId: Hex) => SavedGasFees | undefined; - getSelectedAccount: () => InternalAccount; incomingTransactions?: IncomingTransactionOptions; isMultichainEnabled: boolean; isSimulationEnabled?: () => boolean; @@ -345,7 +344,8 @@ const controllerName = 'TransactionController'; export type AllowedActions = | AddApprovalRequest | NetworkControllerFindNetworkClientIdByChainIdAction - | NetworkControllerGetNetworkClientByIdAction; + | NetworkControllerGetNetworkClientByIdAction + | AccountsControllerGetSelectedAccountAction; /** * The external events available to the {@link TransactionController}. @@ -615,8 +615,6 @@ export class TransactionController extends BaseController< private readonly getPermittedAccounts: (origin?: string) => Promise; - private readonly getSelectedAccount: () => InternalAccount; - private readonly getExternalPendingTransactions: ( address: string, chainId?: string, @@ -734,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.getSelectedAccount - Gets 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. @@ -762,7 +759,6 @@ export class TransactionController extends BaseController< getNetworkState, getPermittedAccounts, getSavedGasFees, - getSelectedAccount, incomingTransactions = {}, isMultichainEnabled = false, isSimulationEnabled, @@ -803,7 +799,6 @@ export class TransactionController extends BaseController< this.getGasFeeEstimates = getGasFeeEstimates || (() => Promise.resolve({} as GasFeeState)); this.getPermittedAccounts = getPermittedAccounts; - this.getSelectedAccount = getSelectedAccount; this.getExternalPendingTransactions = getExternalPendingTransactions ?? (() => []); this.securityProviderRequest = securityProviderRequest; @@ -1036,7 +1031,8 @@ export class TransactionController extends BaseController< if (origin) { await validateTransactionOrigin( await this.getPermittedAccounts(origin), - this.getSelectedAccount().address, + this.messagingSystem.call('AccountsController:getSelectedAccount') + .address, txParams.from, origin, ); @@ -3431,7 +3427,8 @@ export class TransactionController extends BaseController< }): IncomingTransactionHelper { const incomingTransactionHelper = new IncomingTransactionHelper({ blockTracker, - getCurrentAccount: this.getSelectedAccount, + getCurrentAccount: () => + this.messagingSystem.call('AccountsController:getSelectedAccount'), getLastFetchedBlockNumbers: () => this.state.lastFetchedBlockNumbers, getChainId: chainId ? () => chainId : this.getChainId.bind(this), isEnabled: this.#incomingTransactionOptions.isEnabled, diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index 91349913c8..c47378727d 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -1,4 +1,5 @@ import type { TypedTransaction } from '@ethereumjs/tx'; +import type { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller'; import type { ApprovalControllerActions, ApprovalControllerEvents, @@ -61,7 +62,8 @@ import * as etherscanUtils from './utils/etherscan'; type UnrestrictedControllerMessenger = ControllerMessenger< | NetworkControllerActions | ApprovalControllerActions - | TransactionControllerActions, + | TransactionControllerActions + | AccountsControllerGetSelectedAccountAction, | NetworkControllerEvents | ApprovalControllerEvents | TransactionControllerEvents @@ -188,10 +190,20 @@ const setupController = async ( 'ApprovalController:addRequest', 'NetworkController:getNetworkClientById', 'NetworkController:findNetworkClientIdByChainId', + 'AccountsController:getSelectedAccount', ], allowedEvents: ['NetworkController:stateChange'], }); + const mockSelectedAccountCall = jest + .fn() + .mockReturnValue(createMockInternalAccount({ address: '0xdeadbeef' })); + + unrestrictedMessenger.registerActionHandler( + 'AccountsController:getSelectedAccount', + mockSelectedAccountCall, + ); + const options = { blockTracker, disableHistory: false, @@ -230,6 +242,7 @@ const setupController = async ( approvalController, networkController, messenger, + mockSelectedAccountCall, }; }; @@ -841,12 +854,16 @@ describe('TransactionController Integration', () => { ], }); - const { approvalController, networkController, transactionController } = - await setupController({ - isMultichainEnabled: true, - getPermittedAccounts: async () => [ACCOUNT_MOCK], - getSelectedAccount: () => INTERNAL_ACCOUNT_MOCK, - }); + const { + approvalController, + networkController, + transactionController, + mockSelectedAccountCall, + } = await setupController({ + isMultichainEnabled: true, + getPermittedAccounts: async () => [ACCOUNT_MOCK], + }); + mockSelectedAccountCall.mockReturnValue(INTERNAL_ACCOUNT_MOCK); const otherNetworkClientIdOnGoerli = await networkController.upsertNetworkConfiguration( { @@ -922,12 +939,15 @@ describe('TransactionController Integration', () => { buildEthGetTransactionReceiptRequestMock('0x2', '0x2', '0x4'), ], }); - const { approvalController, transactionController } = - await setupController({ - isMultichainEnabled: true, - getPermittedAccounts: async () => [ACCOUNT_MOCK], - getSelectedAccount: () => INTERNAL_ACCOUNT_MOCK, - }); + const { + approvalController, + transactionController, + mockSelectedAccountCall, + } = await setupController({ + isMultichainEnabled: true, + getPermittedAccounts: async () => [ACCOUNT_MOCK], + }); + mockSelectedAccountCall.mockReturnValue(INTERNAL_ACCOUNT_MOCK); const addTx1 = await transactionController.addTransaction( { @@ -1187,11 +1207,14 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { networkController, transactionController } = - await setupController({ - getSelectedAccount: () => selectedAccountMock, - isMultichainEnabled: true, - }); + const { + networkController, + transactionController, + mockSelectedAccountCall, + } = await setupController({ + isMultichainEnabled: true, + }); + mockSelectedAccountCall.mockReturnValue(selectedAccountMock); const expectedLastFetchedBlockNumbers: Record = {}; const expectedTransactions: Partial[] = []; @@ -1274,9 +1297,10 @@ describe('TransactionController Integration', () => { ) .reply(200, ETHERSCAN_TRANSACTION_RESPONSE_MOCK); - const { transactionController } = await setupController({ - getSelectedAccount: () => selectedAccountMock, - }); + const { transactionController, mockSelectedAccountCall } = + await setupController({}); + + mockSelectedAccountCall.mockReturnValue(selectedAccountMock); transactionController.startIncomingTransactionPolling(); @@ -1367,11 +1391,14 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { networkController, transactionController } = - await setupController({ - getSelectedAccount: () => selectedAccountMock, - isMultichainEnabled: true, - }); + const { + networkController, + transactionController, + mockSelectedAccountCall, + } = await setupController({ + isMultichainEnabled: true, + }); + mockSelectedAccountCall.mockReturnValue(selectedAccountMock); const otherGoerliClientNetworkClientId = await networkController.upsertNetworkConfiguration( @@ -1466,10 +1493,12 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { networkController, transactionController } = - await setupController({ - getSelectedAccount: () => selectedAccountMock, - }); + const { + networkController, + transactionController, + mockSelectedAccountCall, + } = await setupController({}); + mockSelectedAccountCall.mockReturnValue(selectedAccountMock); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -1513,9 +1542,10 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { transactionController } = await setupController({ - getSelectedAccount: () => selectedAccountMock, - }); + const { transactionController, mockSelectedAccountCall } = + await setupController({}); + + mockSelectedAccountCall.mockReturnValue(selectedAccountMock); mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( @@ -1552,10 +1582,12 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { networkController, transactionController } = - await setupController({ - getSelectedAccount: () => selectedAccountMock, - }); + const { + networkController, + transactionController, + mockSelectedAccountCall, + } = await setupController({}); + mockSelectedAccountCall.mockReturnValue(selectedAccountMock); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -1599,11 +1631,14 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { networkController, transactionController } = - await setupController({ - getSelectedAccount: () => selectedAccountMock, - isMultichainEnabled: true, - }); + const { + networkController, + transactionController, + mockSelectedAccountCall, + } = await setupController({ + isMultichainEnabled: true, + }); + mockSelectedAccountCall.mockReturnValue(selectedAccountMock); const expectedLastFetchedBlockNumbers: Record = {}; const expectedTransactions: Partial[] = []; @@ -1668,9 +1703,9 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { transactionController } = await setupController({ - getSelectedAccount: () => selectedAccountMock, - }); + const { transactionController, mockSelectedAccountCall } = + await setupController({}); + mockSelectedAccountCall.mockReturnValue(selectedAccountMock); mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts index e44a2f6b2c..b39627cc98 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts @@ -1,4 +1,4 @@ -import type { InternalAccount } from '@metamask/keyring-api'; +import type { AccountsController } from '@metamask/accounts-controller'; import type { BlockTracker } from '@metamask/network-controller'; import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; @@ -36,7 +36,9 @@ export class IncomingTransactionHelper { #blockTracker: BlockTracker; - #getCurrentAccount: () => InternalAccount; + #getCurrentAccount: () => ReturnType< + AccountsController['getSelectedAccount'] + >; #getLastFetchedBlockNumbers: () => Record; @@ -73,7 +75,9 @@ export class IncomingTransactionHelper { updateTransactions, }: { blockTracker: BlockTracker; - getCurrentAccount: () => InternalAccount; + getCurrentAccount: () => ReturnType< + AccountsController['getSelectedAccount'] + >; getLastFetchedBlockNumbers: () => Record; getLocalTransactions?: () => TransactionMeta[]; getChainId: () => Hex; diff --git a/packages/transaction-controller/tsconfig.build.json b/packages/transaction-controller/tsconfig.build.json index 9a78dab46a..648111f91b 100644 --- a/packages/transaction-controller/tsconfig.build.json +++ b/packages/transaction-controller/tsconfig.build.json @@ -6,6 +6,7 @@ "rootDir": "./src" }, "references": [ + { "path": "../accounts-controller/tsconfig.build.json" }, { "path": "../approval-controller/tsconfig.build.json" }, { "path": "../base-controller/tsconfig.build.json" }, { "path": "../controller-utils/tsconfig.build.json" }, diff --git a/packages/transaction-controller/tsconfig.json b/packages/transaction-controller/tsconfig.json index 52940e5592..bf67c43710 100644 --- a/packages/transaction-controller/tsconfig.json +++ b/packages/transaction-controller/tsconfig.json @@ -5,6 +5,7 @@ "target": "ES2022" }, "references": [ + { "path": "../accounts-controller" }, { "path": "../approval-controller" }, { "path": "../base-controller" }, { "path": "../controller-utils" }, diff --git a/yarn.lock b/yarn.lock index d392449a33..4378844987 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2463,7 +2463,7 @@ __metadata: languageName: node linkType: hard -"@metamask/keyring-api@npm:6.4.0, @metamask/keyring-api@npm:^6.3.1, @metamask/keyring-api@npm:^6.4.0": +"@metamask/keyring-api@npm:^6.3.1, @metamask/keyring-api@npm:^6.4.0": version: 6.4.0 resolution: "@metamask/keyring-api@npm:6.4.0" dependencies: @@ -3142,6 +3142,7 @@ __metadata: "@ethersproject/abi": ^5.7.0 "@ethersproject/contracts": ^5.7.0 "@ethersproject/providers": ^5.7.0 + "@metamask/accounts-controller": ^16.0.0 "@metamask/approval-controller": ^7.0.0 "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^6.0.0 @@ -3149,7 +3150,7 @@ __metadata: "@metamask/eth-query": ^4.0.0 "@metamask/ethjs-provider-http": ^0.3.0 "@metamask/gas-fee-controller": ^17.0.0 - "@metamask/keyring-api": 6.4.0 + "@metamask/keyring-api": ^6.4.0 "@metamask/metamask-eth-abis": ^3.1.1 "@metamask/network-controller": ^19.0.0 "@metamask/nonce-tracker": ^5.0.0 @@ -3175,6 +3176,7 @@ __metadata: uuid: ^8.3.2 peerDependencies: "@babel/runtime": ^7.23.9 + "@metamask/accounts-controller": ^16.0.0 "@metamask/approval-controller": ^7.0.0 "@metamask/gas-fee-controller": ^17.0.0 "@metamask/network-controller": ^19.0.0 From a4f995c090cca0804599c2ccfdb83d97a2d84296 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 6 Jun 2024 22:18:33 +0800 Subject: [PATCH 06/10] fix: test --- .../src/TransactionController.test.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 1e2973f6f6..ac64324b18 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -17,6 +17,7 @@ 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, @@ -566,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[0]>; network?: MockNetwork; @@ -580,6 +583,7 @@ describe('TransactionController', () => { typeof mockAddTransactionApprovalRequest >[1]; }; + selectedAccount?: InternalAccount; } = {}) { const unrestrictedMessenger: UnrestrictedControllerMessenger = new ControllerMessenger(); @@ -624,12 +628,10 @@ describe('TransactionController', () => { allowedEvents: [], }); - const mockSelectedAccountCall = jest - .fn() - .mockReturnValue(INTERNAL_ACCOUNT_MOCK); + const mockGetSelectedAccount = jest.fn().mockReturnValue(selectedAccount); unrestrictedMessenger.registerActionHandler( 'AccountsController:getSelectedAccount', - mockSelectedAccountCall, + mockGetSelectedAccount, ); const controller = new TransactionController({ @@ -641,7 +643,7 @@ describe('TransactionController', () => { controller, messenger: unrestrictedMessenger, mockTransactionApprovalRequest, - mockSelectedAccountCall, + mockGetSelectedAccount, }; } From ee06e6089ec6ea59e482f033816b081cb26fac7b Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 6 Jun 2024 23:02:43 +0800 Subject: [PATCH 07/10] feat: add private method for get selected account using the messenger --- .../src/TransactionController.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 7046757d96..27babb1317 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -2,7 +2,10 @@ 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 { + AccountsControllerGetSelectedAccountAction, + AccountsController, +} from '@metamask/accounts-controller'; import type { AcceptResultCallbacks, AddApprovalRequest, @@ -634,6 +637,10 @@ export class TransactionController extends BaseController< private readonly signAbortCallbacks: Map void> = new Map(); + readonly #getSelectedAccount: () => ReturnType< + AccountsController['getSelectedAccount'] + >; + #transactionHistoryLimit: number; #isSimulationEnabled: () => boolean; @@ -784,6 +791,8 @@ export class TransactionController extends BaseController< }); this.messagingSystem = messenger; + this.#getSelectedAccount = () => + this.messagingSystem.call('AccountsController:getSelectedAccount'); this.getNetworkState = getNetworkState; this.isSendFlowHistoryDisabled = disableSendFlowHistory ?? false; this.isHistoryDisabled = disableHistory ?? false; @@ -1031,8 +1040,7 @@ export class TransactionController extends BaseController< if (origin) { await validateTransactionOrigin( await this.getPermittedAccounts(origin), - this.messagingSystem.call('AccountsController:getSelectedAccount') - .address, + this.#getSelectedAccount().address, txParams.from, origin, ); @@ -3426,8 +3434,7 @@ export class TransactionController extends BaseController< }): IncomingTransactionHelper { const incomingTransactionHelper = new IncomingTransactionHelper({ blockTracker, - getCurrentAccount: () => - this.messagingSystem.call('AccountsController:getSelectedAccount'), + getCurrentAccount: () => this.#getSelectedAccount(), getLastFetchedBlockNumbers: () => this.state.lastFetchedBlockNumbers, getChainId: chainId ? () => chainId : this.getChainId.bind(this), isEnabled: this.#incomingTransactionOptions.isEnabled, From fb0ea808797f9f3049c67e973472e4e250ea71ca Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 6 Jun 2024 23:03:04 +0800 Subject: [PATCH 08/10] refactor: rename --- .../TransactionControllerIntegration.test.ts | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index c47378727d..57ec97930f 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -195,13 +195,13 @@ const setupController = async ( allowedEvents: ['NetworkController:stateChange'], }); - const mockSelectedAccountCall = jest + const mockGetSelectedAccount = jest .fn() .mockReturnValue(createMockInternalAccount({ address: '0xdeadbeef' })); unrestrictedMessenger.registerActionHandler( 'AccountsController:getSelectedAccount', - mockSelectedAccountCall, + mockGetSelectedAccount, ); const options = { @@ -242,7 +242,7 @@ const setupController = async ( approvalController, networkController, messenger, - mockSelectedAccountCall, + mockGetSelectedAccount, }; }; @@ -858,12 +858,12 @@ describe('TransactionController Integration', () => { approvalController, networkController, transactionController, - mockSelectedAccountCall, + mockGetSelectedAccount, } = await setupController({ isMultichainEnabled: true, getPermittedAccounts: async () => [ACCOUNT_MOCK], }); - mockSelectedAccountCall.mockReturnValue(INTERNAL_ACCOUNT_MOCK); + mockGetSelectedAccount.mockReturnValue(INTERNAL_ACCOUNT_MOCK); const otherNetworkClientIdOnGoerli = await networkController.upsertNetworkConfiguration( { @@ -942,12 +942,12 @@ describe('TransactionController Integration', () => { const { approvalController, transactionController, - mockSelectedAccountCall, + mockGetSelectedAccount, } = await setupController({ isMultichainEnabled: true, getPermittedAccounts: async () => [ACCOUNT_MOCK], }); - mockSelectedAccountCall.mockReturnValue(INTERNAL_ACCOUNT_MOCK); + mockGetSelectedAccount.mockReturnValue(INTERNAL_ACCOUNT_MOCK); const addTx1 = await transactionController.addTransaction( { @@ -1210,11 +1210,11 @@ describe('TransactionController Integration', () => { const { networkController, transactionController, - mockSelectedAccountCall, + mockGetSelectedAccount, } = await setupController({ isMultichainEnabled: true, }); - mockSelectedAccountCall.mockReturnValue(selectedAccountMock); + mockGetSelectedAccount.mockReturnValue(selectedAccountMock); const expectedLastFetchedBlockNumbers: Record = {}; const expectedTransactions: Partial[] = []; @@ -1297,10 +1297,10 @@ describe('TransactionController Integration', () => { ) .reply(200, ETHERSCAN_TRANSACTION_RESPONSE_MOCK); - const { transactionController, mockSelectedAccountCall } = + const { transactionController, mockGetSelectedAccount } = await setupController({}); - mockSelectedAccountCall.mockReturnValue(selectedAccountMock); + mockGetSelectedAccount.mockReturnValue(selectedAccountMock); transactionController.startIncomingTransactionPolling(); @@ -1394,11 +1394,11 @@ describe('TransactionController Integration', () => { const { networkController, transactionController, - mockSelectedAccountCall, + mockGetSelectedAccount, } = await setupController({ isMultichainEnabled: true, }); - mockSelectedAccountCall.mockReturnValue(selectedAccountMock); + mockGetSelectedAccount.mockReturnValue(selectedAccountMock); const otherGoerliClientNetworkClientId = await networkController.upsertNetworkConfiguration( @@ -1496,9 +1496,9 @@ describe('TransactionController Integration', () => { const { networkController, transactionController, - mockSelectedAccountCall, + mockGetSelectedAccount, } = await setupController({}); - mockSelectedAccountCall.mockReturnValue(selectedAccountMock); + mockGetSelectedAccount.mockReturnValue(selectedAccountMock); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -1542,10 +1542,10 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { transactionController, mockSelectedAccountCall } = + const { transactionController, mockGetSelectedAccount } = await setupController({}); - mockSelectedAccountCall.mockReturnValue(selectedAccountMock); + mockGetSelectedAccount.mockReturnValue(selectedAccountMock); mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( @@ -1585,9 +1585,9 @@ describe('TransactionController Integration', () => { const { networkController, transactionController, - mockSelectedAccountCall, + mockGetSelectedAccount, } = await setupController({}); - mockSelectedAccountCall.mockReturnValue(selectedAccountMock); + mockGetSelectedAccount.mockReturnValue(selectedAccountMock); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -1634,11 +1634,11 @@ describe('TransactionController Integration', () => { const { networkController, transactionController, - mockSelectedAccountCall, + mockGetSelectedAccount, } = await setupController({ isMultichainEnabled: true, }); - mockSelectedAccountCall.mockReturnValue(selectedAccountMock); + mockGetSelectedAccount.mockReturnValue(selectedAccountMock); const expectedLastFetchedBlockNumbers: Record = {}; const expectedTransactions: Partial[] = []; @@ -1703,9 +1703,9 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { transactionController, mockSelectedAccountCall } = + const { transactionController, mockGetSelectedAccount } = await setupController({}); - mockSelectedAccountCall.mockReturnValue(selectedAccountMock); + mockGetSelectedAccount.mockReturnValue(selectedAccountMock); mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( From a0f55fe142c67eee055a3dbff8525d0bf7e631f7 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 6 Jun 2024 23:10:19 +0800 Subject: [PATCH 09/10] refactor: add mock object in constructor of setup --- .../TransactionControllerIntegration.test.ts | 128 ++++++++---------- 1 file changed, 59 insertions(+), 69 deletions(-) diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index 57ec97930f..e4ccee1d49 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -143,6 +143,11 @@ const setupController = async ( givenOptions: Partial< ConstructorParameters[0] > = {}, + mockData: { + selectedAccount?: InternalAccount; + } = { + selectedAccount: createMockInternalAccount({ address: '0xdeadbeef' }), + }, ) => { // Mainnet network must be mocked for NetworkController instantiation mockNetwork({ @@ -197,7 +202,7 @@ const setupController = async ( const mockGetSelectedAccount = jest .fn() - .mockReturnValue(createMockInternalAccount({ address: '0xdeadbeef' })); + .mockReturnValue(mockData.selectedAccount); unrestrictedMessenger.registerActionHandler( 'AccountsController:getSelectedAccount', @@ -221,8 +226,6 @@ const setupController = async ( getNetworkClientRegistry: networkController.getNetworkClientRegistry.bind(networkController), getPermittedAccounts: async () => [ACCOUNT_MOCK], - getSelectedAccount: () => - createMockInternalAccount({ address: '0xdeadbeef' }), hooks: {}, isMultichainEnabled: false, messenger, @@ -854,16 +857,14 @@ describe('TransactionController Integration', () => { ], }); - const { - approvalController, - networkController, - transactionController, - mockGetSelectedAccount, - } = await setupController({ - isMultichainEnabled: true, - getPermittedAccounts: async () => [ACCOUNT_MOCK], - }); - mockGetSelectedAccount.mockReturnValue(INTERNAL_ACCOUNT_MOCK); + const { approvalController, networkController, transactionController } = + await setupController( + { + isMultichainEnabled: true, + getPermittedAccounts: async () => [ACCOUNT_MOCK], + }, + { selectedAccount: INTERNAL_ACCOUNT_MOCK }, + ); const otherNetworkClientIdOnGoerli = await networkController.upsertNetworkConfiguration( { @@ -939,15 +940,14 @@ describe('TransactionController Integration', () => { buildEthGetTransactionReceiptRequestMock('0x2', '0x2', '0x4'), ], }); - const { - approvalController, - transactionController, - mockGetSelectedAccount, - } = await setupController({ - isMultichainEnabled: true, - getPermittedAccounts: async () => [ACCOUNT_MOCK], - }); - mockGetSelectedAccount.mockReturnValue(INTERNAL_ACCOUNT_MOCK); + const { approvalController, transactionController } = + await setupController( + { + isMultichainEnabled: true, + getPermittedAccounts: async () => [ACCOUNT_MOCK], + }, + { selectedAccount: INTERNAL_ACCOUNT_MOCK }, + ); const addTx1 = await transactionController.addTransaction( { @@ -1207,14 +1207,13 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { - networkController, - transactionController, - mockGetSelectedAccount, - } = await setupController({ - isMultichainEnabled: true, - }); - mockGetSelectedAccount.mockReturnValue(selectedAccountMock); + const { networkController, transactionController } = + await setupController( + { + isMultichainEnabled: true, + }, + { selectedAccount: selectedAccountMock }, + ); const expectedLastFetchedBlockNumbers: Record = {}; const expectedTransactions: Partial[] = []; @@ -1297,10 +1296,10 @@ describe('TransactionController Integration', () => { ) .reply(200, ETHERSCAN_TRANSACTION_RESPONSE_MOCK); - const { transactionController, mockGetSelectedAccount } = - await setupController({}); - - mockGetSelectedAccount.mockReturnValue(selectedAccountMock); + const { transactionController } = await setupController( + {}, + { selectedAccount: selectedAccountMock }, + ); transactionController.startIncomingTransactionPolling(); @@ -1391,14 +1390,13 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { - networkController, - transactionController, - mockGetSelectedAccount, - } = await setupController({ - isMultichainEnabled: true, - }); - mockGetSelectedAccount.mockReturnValue(selectedAccountMock); + const { networkController, transactionController } = + await setupController( + { + isMultichainEnabled: true, + }, + { selectedAccount: selectedAccountMock }, + ); const otherGoerliClientNetworkClientId = await networkController.upsertNetworkConfiguration( @@ -1493,12 +1491,8 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { - networkController, - transactionController, - mockGetSelectedAccount, - } = await setupController({}); - mockGetSelectedAccount.mockReturnValue(selectedAccountMock); + const { networkController, transactionController } = + await setupController({}, { selectedAccount: selectedAccountMock }); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -1542,10 +1536,10 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { transactionController, mockGetSelectedAccount } = - await setupController({}); - - mockGetSelectedAccount.mockReturnValue(selectedAccountMock); + const { transactionController } = await setupController( + {}, + { selectedAccount: selectedAccountMock }, + ); mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( @@ -1582,12 +1576,8 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { - networkController, - transactionController, - mockGetSelectedAccount, - } = await setupController({}); - mockGetSelectedAccount.mockReturnValue(selectedAccountMock); + const { networkController, transactionController } = + await setupController({}, { selectedAccount: selectedAccountMock }); const networkClients = networkController.getNetworkClientRegistry(); const networkClientIds = Object.keys(networkClients); @@ -1631,14 +1621,13 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { - networkController, - transactionController, - mockGetSelectedAccount, - } = await setupController({ - isMultichainEnabled: true, - }); - mockGetSelectedAccount.mockReturnValue(selectedAccountMock); + const { networkController, transactionController } = + await setupController( + { + isMultichainEnabled: true, + }, + { selectedAccount: selectedAccountMock }, + ); const expectedLastFetchedBlockNumbers: Record = {}; const expectedTransactions: Partial[] = []; @@ -1703,9 +1692,10 @@ describe('TransactionController Integration', () => { address: selectedAddress, }); - const { transactionController, mockGetSelectedAccount } = - await setupController({}); - mockGetSelectedAccount.mockReturnValue(selectedAccountMock); + const { transactionController } = await setupController( + {}, + { selectedAccount: selectedAccountMock }, + ); mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( From 31141a5109a70e72c59a90f6889885bcd46833f9 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Fri, 7 Jun 2024 17:11:10 +0800 Subject: [PATCH 10/10] refactor: to method --- .../src/TransactionController.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 27babb1317..ecbb49a4b3 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -2,10 +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, - AccountsController, -} from '@metamask/accounts-controller'; +import type { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller'; import type { AcceptResultCallbacks, AddApprovalRequest, @@ -637,10 +634,6 @@ export class TransactionController extends BaseController< private readonly signAbortCallbacks: Map void> = new Map(); - readonly #getSelectedAccount: () => ReturnType< - AccountsController['getSelectedAccount'] - >; - #transactionHistoryLimit: number; #isSimulationEnabled: () => boolean; @@ -791,8 +784,6 @@ export class TransactionController extends BaseController< }); this.messagingSystem = messenger; - this.#getSelectedAccount = () => - this.messagingSystem.call('AccountsController:getSelectedAccount'); this.getNetworkState = getNetworkState; this.isSendFlowHistoryDisabled = disableSendFlowHistory ?? false; this.isHistoryDisabled = disableHistory ?? false; @@ -3849,4 +3840,8 @@ export class TransactionController extends BaseController< ).configuration.type === NetworkClientType.Custom ); } + + #getSelectedAccount() { + return this.messagingSystem.call('AccountsController:getSelectedAccount'); + } }