From f9be3fd37edf87a735134b7cb50b5c6d09c3e956 Mon Sep 17 00:00:00 2001 From: Vinicius Stevam Date: Thu, 12 Jun 2025 10:32:52 +0100 Subject: [PATCH 1/2] use approvalType.TransactionBatch --- .../src/utils/batch.test.ts | 238 +++++------------- .../transaction-controller/src/utils/batch.ts | 4 +- 2 files changed, 66 insertions(+), 176 deletions(-) diff --git a/packages/transaction-controller/src/utils/batch.test.ts b/packages/transaction-controller/src/utils/batch.test.ts index 380f0035414..c1f5a73292a 100644 --- a/packages/transaction-controller/src/utils/batch.test.ts +++ b/packages/transaction-controller/src/utils/batch.test.ts @@ -1,4 +1,5 @@ import { ORIGIN_METAMASK, type AddResult } from '@metamask/approval-controller'; +import { ApprovalType } from '@metamask/controller-utils'; import { rpcErrors, errorCodes } from '@metamask/rpc-errors'; import { @@ -27,6 +28,7 @@ import { TransactionType, GasFeeEstimateLevel, GasFeeEstimateType, + TransactionStatus, } from '..'; import { flushPromises } from '../../../../tests/helpers'; import { DefaultGasFeeFlow } from '../gas-flows/DefaultGasFeeFlow'; @@ -64,6 +66,7 @@ const MESSENGER_MOCK = { } as unknown as TransactionControllerMessenger; const NETWORK_CLIENT_ID_MOCK = 'testNetworkClientId'; const PUBLIC_KEY_MOCK = '0x112233'; +const BATCH_ID_MOCK = '0x654321'; const BATCH_ID_CUSTOM_MOCK = '0x123456'; const GET_ETH_QUERY_MOCK = jest.fn(); const GET_INTERNAL_ACCOUNTS_MOCK = jest.fn().mockReturnValue([]); @@ -95,6 +98,23 @@ const TRANSACTION_BATCH_PARAMS_MOCK = { value: VALUE_MOCK, } as TransactionBatchSingleRequest['params']; +const ADD_APPROVAL_REQUEST_MOCK = { + id: expect.any(String), + origin: ORIGIN_MOCK, + requestData: { txBatchId: expect.any(String) }, + expectsResult: true, + type: ApprovalType.TransactionBatch, +}; + +const TRANSACTIONS_BATCH_MOCK = [ + { + params: TRANSACTION_BATCH_PARAMS_MOCK, + }, + { + params: TRANSACTION_BATCH_PARAMS_MOCK, + }, +]; + /** * Mocks the `ApprovalController:addRequest` action for the `requestApproval` function in `batch.ts`. * @@ -286,6 +306,8 @@ describe('Batch Utils', () => { gasLimit: GAS_TOTAL_MOCK, }); + doesChainSupportEIP7702Mock.mockReturnValue(true); + request = { addTransaction: addTransactionMock, getChainId: getChainIdMock, @@ -300,22 +322,7 @@ describe('Batch Utils', () => { networkClientId: NETWORK_CLIENT_ID_MOCK, origin: ORIGIN_MOCK, requireApproval: true, - transactions: [ - { - params: { - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }, - }, - { - params: { - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }, - }, - ], + transactions: TRANSACTIONS_BATCH_MOCK, disable7702: false, disableHook: false, disableSequential: false, @@ -329,8 +336,6 @@ describe('Batch Utils', () => { }); it('returns generated batch ID', async () => { - doesChainSupportEIP7702Mock.mockReturnValue(true); - isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ delegationAddress: undefined, isSupported: true, @@ -341,20 +346,12 @@ describe('Batch Utils', () => { result: Promise.resolve(''), }); - generateEIP7702BatchTransactionMock.mockReturnValueOnce({ - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }); - const result = await addTransactionBatch(request); expect(result.batchId).toMatch(/^0x[0-9a-f]{32}$/u); }); it('returns provided batch ID', async () => { - doesChainSupportEIP7702Mock.mockReturnValue(true); - isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ delegationAddress: undefined, isSupported: true, @@ -365,12 +362,6 @@ describe('Batch Utils', () => { result: Promise.resolve(''), }); - generateEIP7702BatchTransactionMock.mockReturnValueOnce({ - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }); - request.request.batchId = BATCH_ID_CUSTOM_MOCK; const result = await addTransactionBatch(request); @@ -379,8 +370,6 @@ describe('Batch Utils', () => { }); it('adds generated EIP-7702 transaction', async () => { - doesChainSupportEIP7702Mock.mockReturnValue(true); - isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ delegationAddress: undefined, isSupported: true, @@ -391,11 +380,9 @@ describe('Batch Utils', () => { result: Promise.resolve(''), }); - generateEIP7702BatchTransactionMock.mockReturnValueOnce({ - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }); + generateEIP7702BatchTransactionMock.mockReturnValueOnce( + TRANSACTION_BATCH_PARAMS_MOCK, + ); await addTransactionBatch(request); @@ -416,8 +403,6 @@ describe('Batch Utils', () => { }); it('uses type 4 transaction if not upgraded', async () => { - doesChainSupportEIP7702Mock.mockReturnValue(true); - isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ delegationAddress: undefined, isSupported: false, @@ -428,11 +413,9 @@ describe('Batch Utils', () => { result: Promise.resolve(''), }); - generateEIP7702BatchTransactionMock.mockReturnValueOnce({ - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }); + generateEIP7702BatchTransactionMock.mockReturnValueOnce( + TRANSACTION_BATCH_PARAMS_MOCK, + ); getEIP7702UpgradeContractAddressMock.mockReturnValueOnce( CONTRACT_ADDRESS_MOCK, @@ -458,8 +441,6 @@ describe('Batch Utils', () => { }); it('passes nested transactions to add transaction', async () => { - doesChainSupportEIP7702Mock.mockReturnValue(true); - isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ delegationAddress: undefined, isSupported: true, @@ -470,12 +451,6 @@ describe('Batch Utils', () => { result: Promise.resolve(''), }); - generateEIP7702BatchTransactionMock.mockReturnValueOnce({ - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }); - await addTransactionBatch(request); expect(addTransactionMock).toHaveBeenCalledTimes(1); @@ -483,24 +458,14 @@ describe('Batch Utils', () => { expect.any(Object), expect.objectContaining({ nestedTransactions: [ - expect.objectContaining({ - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }), - expect.objectContaining({ - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }), + expect.objectContaining(TRANSACTION_BATCH_PARAMS_MOCK), + expect.objectContaining(TRANSACTION_BATCH_PARAMS_MOCK), ], }), ); }); it('determines transaction type for nested transactions', async () => { - doesChainSupportEIP7702Mock.mockReturnValue(true); - isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ delegationAddress: undefined, isSupported: true, @@ -511,12 +476,6 @@ describe('Batch Utils', () => { result: Promise.resolve(''), }); - generateEIP7702BatchTransactionMock.mockReturnValueOnce({ - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }); - determineTransactionTypeMock .mockResolvedValueOnce({ type: TransactionType.tokenMethodSafeTransferFrom, @@ -552,15 +511,12 @@ describe('Batch Utils', () => { }); it('throws if no public key', async () => { - doesChainSupportEIP7702Mock.mockReturnValue(true); - await expect( addTransactionBatch({ ...request, publicKeyEIP7702: undefined }), ).rejects.toThrow(rpcErrors.internal(ERROR_MESSGE_PUBLIC_KEY)); }); it('throws if account upgraded to unsupported contract', async () => { - doesChainSupportEIP7702Mock.mockReturnValue(true); isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ delegationAddress: CONTRACT_ADDRESS_MOCK, isSupported: false, @@ -572,8 +528,6 @@ describe('Batch Utils', () => { }); it('throws if account not upgraded and no upgrade address', async () => { - doesChainSupportEIP7702Mock.mockReturnValue(true); - isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ delegationAddress: undefined, isSupported: false, @@ -597,8 +551,6 @@ describe('Batch Utils', () => { }); it('adds security alert ID to transaction', async () => { - doesChainSupportEIP7702Mock.mockReturnValue(true); - isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ delegationAddress: undefined, isSupported: true, @@ -609,12 +561,6 @@ describe('Batch Utils', () => { result: Promise.resolve(''), }); - generateEIP7702BatchTransactionMock.mockReturnValueOnce({ - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }); - request.request.securityAlertId = SECURITY_ALERT_ID_MOCK; await addTransactionBatch(request); @@ -632,7 +578,6 @@ describe('Batch Utils', () => { describe('validates security', () => { it('using transaction params', async () => { - doesChainSupportEIP7702Mock.mockReturnValue(true); isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ delegationAddress: undefined, @@ -644,11 +589,9 @@ describe('Batch Utils', () => { result: Promise.resolve(''), }); - generateEIP7702BatchTransactionMock.mockReturnValueOnce({ - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }); + generateEIP7702BatchTransactionMock.mockReturnValueOnce( + TRANSACTION_BATCH_PARAMS_MOCK, + ); const validateSecurityMock = jest.fn(); validateSecurityMock.mockResolvedValueOnce({}); @@ -679,8 +622,6 @@ describe('Batch Utils', () => { }); it('using delegation mock if not upgraded', async () => { - doesChainSupportEIP7702Mock.mockReturnValue(true); - isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ delegationAddress: undefined, isSupported: false, @@ -691,11 +632,9 @@ describe('Batch Utils', () => { result: Promise.resolve(''), }); - generateEIP7702BatchTransactionMock.mockReturnValueOnce({ - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }); + generateEIP7702BatchTransactionMock.mockReturnValueOnce( + TRANSACTION_BATCH_PARAMS_MOCK, + ); getEIP7702UpgradeContractAddressMock.mockReturnValue( CONTRACT_ADDRESS_MOCK, @@ -783,25 +722,16 @@ describe('Batch Utils', () => { ])( 'requests approval for batch transactions $description', async ({ origin, expectedOrigin }) => { - const publishBatchHook = jest.fn(); - addTransactionMock.mockResolvedValueOnce({ transactionMeta: TRANSACTION_META_MOCK, result: Promise.resolve(''), }); - generateEIP7702BatchTransactionMock.mockReturnValueOnce({ - to: TO_MOCK, - data: DATA_MOCK, - value: VALUE_MOCK, - }); - - request.messenger = MESSENGER_MOCK; - addTransactionBatch({ ...request, - publishBatchHook, + publishBatchHook: jest.fn(), request: { ...request.request, origin, disable7702: true }, + messenger: MESSENGER_MOCK, }).catch(() => { // Intentionally empty }); @@ -810,13 +740,7 @@ describe('Batch Utils', () => { expect(MESSENGER_MOCK.call).toHaveBeenCalledWith( 'ApprovalController:addRequest', - expect.objectContaining({ - id: expect.any(String), - origin: expectedOrigin, - requestData: { txBatchId: expect.any(String) }, - expectsResult: true, - type: 'transaction_batch', - }), + { ...ADD_APPROVAL_REQUEST_MOCK, origin: expectedOrigin }, true, ); expect(simulateGasBatchMock).toHaveBeenCalledTimes(1); @@ -891,12 +815,12 @@ describe('Batch Utils', () => { transactions: [ { id: TRANSACTION_ID_MOCK, - params: { data: DATA_MOCK, to: TO_MOCK, value: VALUE_MOCK }, + params: TRANSACTION_BATCH_PARAMS_MOCK, signedTx: TRANSACTION_SIGNATURE_MOCK, }, { id: TRANSACTION_ID_2_MOCK, - params: { data: DATA_MOCK, to: TO_MOCK, value: VALUE_MOCK }, + params: TRANSACTION_BATCH_PARAMS_MOCK, signedTx: TRANSACTION_SIGNATURE_2_MOCK, }, ], @@ -1055,16 +979,12 @@ describe('Batch Utils', () => { transactions: [ { id: TRANSACTION_ID_2_MOCK, - params: { - data: DATA_MOCK, - to: TO_MOCK, - value: VALUE_MOCK, - }, + params: TRANSACTION_BATCH_PARAMS_MOCK, signedTx: TRANSACTION_SIGNATURE_2_MOCK, }, { id: TRANSACTION_ID_MOCK, - params: { data: DATA_MOCK, to: TO_MOCK, value: VALUE_MOCK }, + params: TRANSACTION_BATCH_PARAMS_MOCK, signedTx: TRANSACTION_SIGNATURE_MOCK, }, ], @@ -1396,16 +1316,16 @@ describe('Batch Utils', () => { from: FROM_MOCK, networkClientId: NETWORK_CLIENT_ID_MOCK, transactions: [ - expect.objectContaining({ + { id: TRANSACTION_ID_MOCK, - params: { data: DATA_MOCK, to: TO_MOCK, value: VALUE_MOCK }, + params: TRANSACTION_BATCH_PARAMS_MOCK, signedTx: TRANSACTION_SIGNATURE_MOCK, - }), - expect.objectContaining({ + }, + { id: TRANSACTION_ID_2_MOCK, - params: { data: DATA_MOCK, to: TO_MOCK, value: VALUE_MOCK }, + params: TRANSACTION_BATCH_PARAMS_MOCK, signedTx: TRANSACTION_SIGNATURE_2_MOCK, - }), + }, ], }); }; @@ -1504,13 +1424,7 @@ describe('Batch Utils', () => { expect(MESSENGER_MOCK.call).toHaveBeenCalledWith( 'ApprovalController:addRequest', - expect.objectContaining({ - id: expect.any(String), - origin: ORIGIN_MOCK, - requestData: { txBatchId: expect.any(String) }, - expectsResult: true, - type: 'transaction_batch', - }), + ADD_APPROVAL_REQUEST_MOCK, true, ); @@ -1551,14 +1465,7 @@ describe('Batch Utils', () => { expect(simulateGasBatchMock).toHaveBeenCalledWith({ chainId: CHAIN_ID_MOCK, from: FROM_MOCK, - transactions: [ - { - params: TRANSACTION_BATCH_PARAMS_MOCK, - }, - { - params: TRANSACTION_BATCH_PARAMS_MOCK, - }, - ], + transactions: TRANSACTIONS_BATCH_MOCK, }); expect(getGasFeesMock).toHaveBeenCalledTimes(1); expect(getGasFeesMock).toHaveBeenCalledWith( @@ -1573,16 +1480,9 @@ describe('Batch Utils', () => { txParams: { from: FROM_MOCK, gas: GAS_TOTAL_MOCK }, origin: ORIGIN_MOCK, id: expect.any(String), - status: 'unapproved', + status: TransactionStatus.unapproved, time: expect.any(Number), - transactions: [ - { - params: TRANSACTION_BATCH_PARAMS_MOCK, - }, - { - params: TRANSACTION_BATCH_PARAMS_MOCK, - }, - ], + transactions: TRANSACTIONS_BATCH_MOCK, }, }), ); @@ -1616,45 +1516,35 @@ describe('Batch Utils', () => { expect(MESSENGER_MOCK.call).toHaveBeenCalledWith( 'ApprovalController:addRequest', - expect.objectContaining({ - id: expect.any(String), - origin: ORIGIN_MOCK, - requestData: { txBatchId: expect.any(String) }, - expectsResult: true, - type: 'transaction_batch', - }), + ADD_APPROVAL_REQUEST_MOCK, true, ); expect(updateMock).toHaveBeenCalledTimes(2); expect(updateMock).toHaveBeenCalledWith(expect.any(Function)); + const BATCH_TRANSACTION_MOCK = { + id: BATCH_ID_MOCK, + chainId: CHAIN_ID_MOCK, + transactions: [], + }; // Simulate the state update for adding the batch const state = { - transactionBatches: [ - { id: 'batch1', chainId: '0x1', transactions: [] }, - ], + transactionBatches: [BATCH_TRANSACTION_MOCK], } as unknown as TransactionControllerState; // Simulate adding the batch updateMock.mock.calls[0][0](state); expect(state.transactionBatches).toStrictEqual([ - { id: 'batch1', chainId: '0x1', transactions: [] }, + BATCH_TRANSACTION_MOCK, expect.objectContaining({ id: expect.any(String), chainId: CHAIN_ID_MOCK, gas: GAS_TOTAL_MOCK, from: FROM_MOCK, networkClientId: NETWORK_CLIENT_ID_MOCK, - transactions: [ - { - params: TRANSACTION_BATCH_PARAMS_MOCK, - }, - { - params: TRANSACTION_BATCH_PARAMS_MOCK, - }, - ], + transactions: TRANSACTIONS_BATCH_MOCK, origin: ORIGIN_MOCK, }), ]); @@ -1665,7 +1555,7 @@ describe('Batch Utils', () => { updateMock.mock.calls[1][0](state); expect(state.transactionBatches).toStrictEqual([ - { id: 'batch1', chainId: '0x1', transactions: [] }, + BATCH_TRANSACTION_MOCK, ]); expect(simulateGasBatchMock).toHaveBeenCalledTimes(1); expect(getGasFeesMock).toHaveBeenCalledTimes(1); diff --git a/packages/transaction-controller/src/utils/batch.ts b/packages/transaction-controller/src/utils/batch.ts index 18f6d5402c2..598c043f573 100644 --- a/packages/transaction-controller/src/utils/batch.ts +++ b/packages/transaction-controller/src/utils/batch.ts @@ -2,7 +2,7 @@ import type { AcceptResultCallbacks, AddResult, } from '@metamask/approval-controller'; -import { ORIGIN_METAMASK } from '@metamask/controller-utils'; +import { ApprovalType, ORIGIN_METAMASK } from '@metamask/controller-utils'; import type EthQuery from '@metamask/eth-query'; import type { FetchGasFeeEstimateOptions, @@ -625,7 +625,7 @@ async function requestApproval( ): Promise { const id = String(txBatchMeta.id); const { origin } = txBatchMeta; - const type = 'transaction_batch'; + const type = ApprovalType.TransactionBatch; const requestData = { txBatchId: id }; log('Requesting approval for transaction batch', id); From b8a8c372badcab40b067f41f6aa567251ec11946 Mon Sep 17 00:00:00 2001 From: Vinicius Stevam Date: Thu, 12 Jun 2025 10:42:32 +0100 Subject: [PATCH 2/2] fix lint --- packages/transaction-controller/src/utils/batch.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/transaction-controller/src/utils/batch.test.ts b/packages/transaction-controller/src/utils/batch.test.ts index c1f5a73292a..a1c7af9d961 100644 --- a/packages/transaction-controller/src/utils/batch.test.ts +++ b/packages/transaction-controller/src/utils/batch.test.ts @@ -578,7 +578,6 @@ describe('Batch Utils', () => { describe('validates security', () => { it('using transaction params', async () => { - isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({ delegationAddress: undefined, isSupported: true,