diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 142f0705157..607282fb579 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -440,13 +440,6 @@ describe('TransactionController', () => { ); const getBalanceChangesMock = jest.mocked(getBalanceChanges); const getGasFeeTokensMock = jest.mocked(getGasFeeTokens); - const createGasFeeTokensResult = ( - gasFeeTokens: GasFeeToken[], - isGasFeeSponsoredAvailable = false, - ): Awaited> => ({ - gasFeeTokens, - isGasFeeSponsoredAvailable, - }); const getTransactionLayer1GasFeeMock = jest.mocked( getTransactionLayer1GasFee, ); @@ -523,12 +516,7 @@ describe('TransactionController', () => { }, updateToInitialState = false, }: { - options?: Omit< - Partial[0]>, - 'hooks' - > & { - hooks?: Partial>; - }; + options?: Partial[0]>; network?: { blockTracker?: BlockTracker; provider?: Provider; @@ -613,6 +601,7 @@ describe('TransactionController', () => { }: Partial = { disableSwaps: false, getPermittedAccounts: async () => [ACCOUNT_MOCK], + hooks: {}, isEIP7702GasFeeTokensEnabled: isEIP7702GasFeeTokensEnabledMock, publicKeyEIP7702: '0x1234', ...givenOptions, @@ -822,7 +811,6 @@ describe('TransactionController', () => { beforeEach(() => { jest.resetAllMocks(); - getGasFeeTokensMock.mockResolvedValue(createGasFeeTokensResult([])); jest.spyOn(Date, 'now').mockImplementation(() => { timeCounter += 1; @@ -976,39 +964,6 @@ describe('TransactionController', () => { ); }); - it('updates transaction batch gas fee estimates when the poller emits a batch update', async () => { - const batchId = BATCH_ID_MOCK; - const { controller } = setupController({ - options: { - state: { - transactionBatches: [{ id: batchId } as never], - }, - }, - }); - const batchUpdateHandler = gasFeePollerMock.hub.on.mock.calls.find( - ([event]) => event === 'transaction-batch-updated', - )?.[1] as (request: { - transactionBatchId: Hex; - gasFeeEstimates?: GasFeeEstimates; - }) => void; - - batchUpdateHandler({ - transactionBatchId: batchId, - gasFeeEstimates: { - type: GasFeeEstimateType.FeeMarket, - } as GasFeeEstimates, - }); - - expect(controller.state.transactionBatches).toContainEqual( - expect.objectContaining({ - id: batchId, - gasFeeEstimates: { - type: GasFeeEstimateType.FeeMarket, - }, - }), - ); - }); - it('provides only test flow if option set', () => { setupController({ options: { @@ -2260,9 +2215,10 @@ describe('TransactionController', () => { }; it('skips balance changes but refreshes gas fee tokens using the wrapped transaction', async () => { - getGasFeeTokensMock.mockResolvedValueOnce( - createGasFeeTokensResult([refreshedGasFeeToken]), - ); + getGasFeeTokensMock.mockResolvedValueOnce({ + gasFeeTokens: [refreshedGasFeeToken], + isGasFeeSponsored: false, + }); shouldResimulateMock.mockReturnValueOnce({ blockTime: 123, resimulate: true, @@ -2272,6 +2228,7 @@ describe('TransactionController', () => { ...TRANSACTION_META_MOCK, gasFeeTokens: [GAS_FEE_TOKEN_MOCK], gasUsed: originalGasUsed, + isGasFeeSponsored: true, selectedGasFeeToken: GAS_FEE_TOKEN_MOCK.tokenAddress, simulationData: SIMULATION_DATA_RESULT_MOCK, status: TransactionStatus.unapproved as const, @@ -2302,6 +2259,8 @@ describe('TransactionController', () => { expect(controller.state.transactions[0]).toMatchObject({ gasFeeTokens: [refreshedGasFeeToken], gasUsed: originalGasUsed, + isExternalSign: true, + isGasFeeSponsored: true, selectedGasFeeToken: GAS_FEE_TOKEN_MOCK.tokenAddress, simulationData: SIMULATION_DATA_RESULT_MOCK, }); @@ -2309,9 +2268,10 @@ describe('TransactionController', () => { it('keeps the selected token during periodic simulation updates', async () => { jest.useFakeTimers(); - getGasFeeTokensMock.mockResolvedValue( - createGasFeeTokensResult([refreshedGasFeeToken]), - ); + getGasFeeTokensMock.mockResolvedValue({ + gasFeeTokens: [refreshedGasFeeToken], + isGasFeeSponsored: false, + }); const transactionMeta = { ...TRANSACTION_META_MOCK, containerTypes: [TransactionContainerType.EnforcedSimulations], @@ -2344,7 +2304,10 @@ describe('TransactionController', () => { }); it('replaces existing tokens with an empty response without clearing the selection', async () => { - getGasFeeTokensMock.mockResolvedValueOnce(createGasFeeTokensResult([])); + getGasFeeTokensMock.mockResolvedValueOnce({ + gasFeeTokens: [], + isGasFeeSponsored: false, + }); shouldResimulateMock.mockReturnValueOnce({ blockTime: 123, resimulate: true, @@ -2392,9 +2355,10 @@ describe('TransactionController', () => { TransactionContainerType.EnforcedSimulations, ); - return createGasFeeTokensResult([ - isWrapped ? refreshedGasFeeToken : staleGasFeeToken, - ]); + return { + gasFeeTokens: [isWrapped ? refreshedGasFeeToken : staleGasFeeToken], + isGasFeeSponsored: false, + }; }); shouldResimulateMock.mockReturnValue({ blockTime: 123, @@ -2445,349 +2409,6 @@ describe('TransactionController', () => { }); }); - describe('with sponsored approval hooks', () => { - it('defaults to local signing without sponsorship when hooks are omitted', async () => { - const { controller } = setupController({ - messengerOptions: { - addTransactionApprovalRequest: { - state: 'approved', - }, - }, - options: { - hooks: {}, - }, - }); - - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await flushPromises(); - - expect(getNonceLockSpy).toHaveBeenCalledTimes(1); - expect(signMock).toHaveBeenCalledTimes(1); - }); - - it('does not refresh sponsorship availability when simulation is disabled', async () => { - const isSponsoredHook = jest - .fn() - .mockResolvedValue({ isSponsored: false }); - const { controller } = setupController({ - messengerOptions: { - addTransactionApprovalRequest: { - state: 'approved', - }, - }, - options: { - hooks: { - isSponsored: isSponsoredHook, - }, - isSimulationEnabled: () => false, - }, - }); - - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await flushPromises(); - - expect(getGasFeeTokensMock).not.toHaveBeenCalled(); - expect(isSponsoredHook).toHaveBeenCalledWith({ - transactionMeta: expect.not.objectContaining({ - isGasFeeSponsoredAvailable: expect.anything(), - }), - }); - }); - - it('calls isSponsored hook with current sponsorship availability before reserving a nonce', async () => { - const callOrder: string[] = []; - - getGasFeeTokensMock - .mockResolvedValueOnce(createGasFeeTokensResult([], false)) - .mockResolvedValue(createGasFeeTokensResult([], true)); - const isSponsoredHook = jest - .fn() - .mockImplementation( - async ({ transactionMeta }): Promise<{ isSponsored: boolean }> => { - callOrder.push('isSponsored'); - expect(transactionMeta.isGasFeeSponsoredAvailable).toBe(true); - expect(getNonceLockSpy).not.toHaveBeenCalled(); - return { isSponsored: false }; - }, - ); - - const shouldSignHook = jest - .fn() - .mockImplementation(async (): Promise<{ shouldSign: boolean }> => { - callOrder.push('shouldSign'); - expect(getNonceLockSpy).not.toHaveBeenCalled(); - return { shouldSign: true }; - }); - - getNonceLockSpy.mockImplementation( - async (): Promise<{ - nextNonce: Hex; - releaseLock: () => Promise; - }> => { - callOrder.push('getNonceLock'); - return { - nextNonce: toHex(NONCE_MOCK), - releaseLock: () => Promise.resolve(), - }; - }, - ); - - const { controller, messenger } = setupController({ - messengerOptions: { - addTransactionApprovalRequest: { - state: 'approved', - }, - }, - options: { - hooks: { - isSponsored: isSponsoredHook, - shouldSign: shouldSignHook, - }, - }, - }); - const statusUpdatedListener = jest.fn(); - messenger.subscribe( - 'TransactionController:transactionStatusUpdated', - statusUpdatedListener, - ); - - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await flushPromises(); - - expect(isSponsoredHook).toHaveBeenCalledTimes(1); - expect(shouldSignHook).toHaveBeenCalledTimes(1); - expect(callOrder).toStrictEqual([ - 'isSponsored', - 'shouldSign', - 'getNonceLock', - ]); - expect(statusUpdatedListener).toHaveBeenCalledTimes(3); - expect(statusUpdatedListener).toHaveBeenNthCalledWith(1, { - transactionMeta: expect.objectContaining({ - status: TransactionStatus.approved, - txParams: expect.objectContaining({ nonce: toHex(NONCE_MOCK) }), - }), - }); - expect(statusUpdatedListener).toHaveBeenNthCalledWith(2, { - transactionMeta: expect.objectContaining({ - status: TransactionStatus.signed, - }), - }); - expect(statusUpdatedListener).toHaveBeenNthCalledWith(3, { - transactionMeta: expect.objectContaining({ - status: TransactionStatus.submitted, - }), - }); - }); - - it('skips shouldSign and nonce reservation when sponsored', async () => { - const isSponsoredHook = jest - .fn() - .mockResolvedValue({ isSponsored: true }); - const shouldSignHook = jest - .fn() - .mockResolvedValue({ shouldSign: true }); - - const { controller } = setupController({ - messengerOptions: { - addTransactionApprovalRequest: { - state: 'approved', - }, - }, - options: { - hooks: { - isSponsored: isSponsoredHook, - publish: async () => ({ transactionHash: TRANSACTION_HASH_MOCK }), - shouldSign: shouldSignHook, - }, - }, - }); - - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await flushPromises(); - - expect(isSponsoredHook).toHaveBeenCalledTimes(1); - expect(shouldSignHook).not.toHaveBeenCalled(); - expect(getNonceLockSpy).not.toHaveBeenCalled(); - }); - - it('skips nonce reservation and preserves an existing nonce when shouldSign resolves false', async () => { - const isSponsoredHook = jest - .fn() - .mockResolvedValue({ isSponsored: false }); - const shouldSignHook = jest - .fn() - .mockResolvedValue({ shouldSign: false }); - - const { controller } = setupController({ - messengerOptions: { - addTransactionApprovalRequest: { - state: 'approved', - }, - }, - options: { - hooks: { - isSponsored: isSponsoredHook, - publish: async () => ({ transactionHash: TRANSACTION_HASH_MOCK }), - shouldSign: shouldSignHook, - }, - }, - }); - - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - nonce: toHex(NONCE_MOCK), - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await flushPromises(); - - expect(isSponsoredHook).toHaveBeenCalledTimes(1); - expect(shouldSignHook).toHaveBeenCalledTimes(1); - expect(getNonceLockSpy).not.toHaveBeenCalled(); - expect(controller.state.transactions[0].txParams.nonce).toBe( - toHex(NONCE_MOCK), - ); - }); - - it('preserves deprecated sponsorship metadata without using it as lifecycle policy', async () => { - const { controller } = setupController({ - messengerOptions: { - addTransactionApprovalRequest: { - state: 'approved', - }, - }, - }); - - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - isGasFeeSponsored: true, - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await flushPromises(); - - expect(controller.state.transactions[0].isGasFeeSponsored).toBe(true); - expect(signMock).toHaveBeenCalledTimes(1); - }); - - it('does not persist hook decisions as transaction metadata', async () => { - const { controller } = setupController({ - messengerOptions: { - addTransactionApprovalRequest: { - state: 'approved', - }, - }, - options: { - hooks: { - isSponsored: jest.fn().mockResolvedValue({ isSponsored: true }), - publish: async () => ({ transactionHash: TRANSACTION_HASH_MOCK }), - shouldSign: jest.fn().mockResolvedValue({ shouldSign: false }), - }, - }, - }); - - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await flushPromises(); - - expect(controller.state.transactions[0]).not.toHaveProperty( - 'isGasFeeSponsored', - ); - expect(controller.state.transactions[0]).not.toHaveProperty( - 'isExternalSign', - ); - }); - - it('still runs beforeSign when shouldSign resolves false', async () => { - const beforeSignHook = jest.fn().mockResolvedValueOnce({}); - - const { controller } = setupController({ - messengerOptions: { - addTransactionApprovalRequest: { - state: 'approved', - }, - }, - options: { - hooks: { - beforeSign: beforeSignHook, - publish: async () => ({ transactionHash: TRANSACTION_HASH_MOCK }), - shouldSign: jest.fn().mockResolvedValue({ shouldSign: false }), - }, - }, - }); - - await controller.addTransaction( - { - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - await flushPromises(); - - expect(beforeSignHook).toHaveBeenCalledTimes(1); - expect(getNonceLockSpy).not.toHaveBeenCalled(); - }); - }); - describe('with beforeSign hook', () => { it('calls beforeSign hook', async () => { const beforeSignHook = jest.fn().mockResolvedValueOnce({}); @@ -2953,9 +2574,10 @@ describe('TransactionController', () => { getGasFeeTokensMock.mockImplementation(async ({ transactionMeta }) => { const isFresh = transactionMeta.txParams.data === '0x2222'; - return createGasFeeTokensResult([ - isFresh ? freshGasFeeToken : staleGasFeeToken, - ]); + return { + gasFeeTokens: [isFresh ? freshGasFeeToken : staleGasFeeToken], + isGasFeeSponsored: isFresh, + }; }); shouldResimulateMock.mockReturnValue({ blockTime: 123, @@ -2996,6 +2618,8 @@ describe('TransactionController', () => { expect(controller.state.transactions[0]).toMatchObject({ gasFeeTokens: [freshGasFeeToken], gasUsed: '0x300', + isExternalSign: true, + isGasFeeSponsored: true, revert: { simulation: freshSimulationRevert }, simulationData: freshSimulationData, txParams: expect.objectContaining({ data: '0x2222' }), @@ -3032,39 +2656,222 @@ describe('TransactionController', () => { await flushPromises(); - expect(getBalanceChangesMock).toHaveBeenCalledTimes(1); - expect(getBalanceChangesMock).toHaveBeenCalledWith( - expect.objectContaining({ - getSimulationConfig: expect.any(Function), - }), - ); - - expect(getSimulationConfigMock).toHaveBeenCalledTimes(1); - expect(getSimulationConfigMock).toHaveBeenCalledWith( - 'https://testurl.com', - { - txMeta: expect.objectContaining({ - txParams: expect.objectContaining({ - from: ACCOUNT_MOCK, - to: ACCOUNT_MOCK, - }), - }), - }, - ); - - expect(controller.state.transactions[0].simulationData).toStrictEqual( - SIMULATION_DATA_RESULT_MOCK, - ); + expect(getBalanceChangesMock).toHaveBeenCalledTimes(1); + expect(getBalanceChangesMock).toHaveBeenCalledWith( + expect.objectContaining({ + getSimulationConfig: expect.any(Function), + }), + ); + + expect(getSimulationConfigMock).toHaveBeenCalledTimes(1); + expect(getSimulationConfigMock).toHaveBeenCalledWith( + 'https://testurl.com', + { + txMeta: expect.objectContaining({ + txParams: expect.objectContaining({ + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }), + }), + }, + ); + + expect(controller.state.transactions[0].simulationData).toStrictEqual( + SIMULATION_DATA_RESULT_MOCK, + ); + }); + + it('with error if simulation disabled', async () => { + getBalanceChangesMock.mockResolvedValueOnce({ + simulationData: SIMULATION_DATA_RESULT_MOCK, + }); + + const { controller } = setupController({ + options: { isSimulationEnabled: () => false }, + }); + + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); + + expect(getBalanceChangesMock).toHaveBeenCalledTimes(0); + expect(controller.state.transactions[0].simulationData).toStrictEqual({ + error: { + code: SimulationErrorCode.Disabled, + message: 'Simulation disabled', + }, + tokenBalanceChanges: [], + }); + }); + + it('passes the transaction meta to the isSimulationEnabled callback', async () => { + const isSimulationEnabled = jest.fn().mockReturnValue(true); + + const { controller } = setupController({ + options: { isSimulationEnabled }, + }); + + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); + + await flushPromises(); + + expect(isSimulationEnabled).toHaveBeenCalledWith( + expect.objectContaining({ id: transactionMeta.id }), + ); + }); + + it('unless approval not required', async () => { + getBalanceChangesMock.mockResolvedValueOnce({ + simulationData: SIMULATION_DATA_RESULT_MOCK, + }); + + const { controller } = setupController(); + + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { requireApproval: false, networkClientId: NETWORK_CLIENT_ID_MOCK }, + ); + + expect(getBalanceChangesMock).toHaveBeenCalledTimes(0); + expect(controller.state.transactions[0].simulationData).toBeUndefined(); + }); + }); + + describe('updates gas fee tokens', () => { + it('by default', async () => { + getGasFeeTokensMock.mockResolvedValueOnce({ + gasFeeTokens: [GAS_FEE_TOKEN_MOCK], + isGasFeeSponsored: false, + }); + + const { controller } = setupController(); + + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); + + await flushPromises(); + + expect(controller.state.transactions[0].gasFeeTokens).toStrictEqual([ + GAS_FEE_TOKEN_MOCK, + ]); + }); + + it('with getSimulationConfig', async () => { + getGasFeeTokensMock.mockResolvedValueOnce({ + gasFeeTokens: [GAS_FEE_TOKEN_MOCK], + isGasFeeSponsored: false, + }); + + const getSimulationConfigMock: GetSimulationConfig = jest + .fn() + .mockResolvedValue({}); + + const { controller } = setupController({ + options: { + getSimulationConfig: getSimulationConfigMock, + }, + }); + + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); + + await flushPromises(); + + expect(getGasFeeTokensMock).toHaveBeenCalledTimes(1); + expect(getGasFeeTokensMock).toHaveBeenCalledWith( + expect.objectContaining({ + getSimulationConfig: getSimulationConfigMock, + }), + ); + + expect(controller.state.transactions[0].gasFeeTokens).toStrictEqual([ + GAS_FEE_TOKEN_MOCK, + ]); + }); + + it('unless approval not required', async () => { + getGasFeeTokensMock.mockResolvedValueOnce({ + gasFeeTokens: [GAS_FEE_TOKEN_MOCK], + isGasFeeSponsored: false, + }); + + const { controller } = setupController(); + + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { requireApproval: false, networkClientId: NETWORK_CLIENT_ID_MOCK }, + ); + + expect(getBalanceChangesMock).toHaveBeenCalledTimes(0); + expect(controller.state.transactions[0].gasFeeTokens).toBeUndefined(); + }); + }); + + describe('updates isGasFeeSponsored', () => { + it('sets isGasFeeSponsored to true when transaction is sponsored', async () => { + getGasFeeTokensMock.mockResolvedValueOnce({ + gasFeeTokens: [GAS_FEE_TOKEN_MOCK], + isGasFeeSponsored: true, + }); + + const { controller } = setupController(); + + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); + + await flushPromises(); + + expect(controller.state.transactions[0].isGasFeeSponsored).toBe(true); }); - it('with error if simulation disabled', async () => { - getBalanceChangesMock.mockResolvedValueOnce({ - simulationData: SIMULATION_DATA_RESULT_MOCK, + it('sets isGasFeeSponsored to false when transaction is not sponsored', async () => { + getGasFeeTokensMock.mockResolvedValueOnce({ + gasFeeTokens: [GAS_FEE_TOKEN_MOCK], + isGasFeeSponsored: false, }); - const { controller } = setupController({ - options: { isSimulationEnabled: () => false }, - }); + const { controller } = setupController(); await controller.addTransaction( { @@ -3076,24 +2883,19 @@ describe('TransactionController', () => { }, ); - expect(getBalanceChangesMock).toHaveBeenCalledTimes(0); - expect(controller.state.transactions[0].simulationData).toStrictEqual({ - error: { - code: SimulationErrorCode.Disabled, - message: 'Simulation disabled', - }, - tokenBalanceChanges: [], - }); - }); + await flushPromises(); - it('passes the transaction meta to the isSimulationEnabled callback', async () => { - const isSimulationEnabled = jest.fn().mockReturnValue(true); + expect(controller.state.transactions[0].isGasFeeSponsored).toBe(false); + }); + it('defaults isGasFeeSponsored to false when gas fee tokens are disabled', async () => { const { controller } = setupController({ - options: { isSimulationEnabled }, + options: { + isEIP7702GasFeeTokensEnabled: () => Promise.resolve(false), + }, }); - const { transactionMeta } = await controller.addTransaction( + await controller.addTransaction( { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK, @@ -3105,14 +2907,13 @@ describe('TransactionController', () => { await flushPromises(); - expect(isSimulationEnabled).toHaveBeenCalledWith( - expect.objectContaining({ id: transactionMeta.id }), - ); + expect(controller.state.transactions[0].isGasFeeSponsored).toBe(false); }); - it('unless approval not required', async () => { - getBalanceChangesMock.mockResolvedValueOnce({ - simulationData: SIMULATION_DATA_RESULT_MOCK, + it('sets isExternalSign to true when transaction is sponsored', async () => { + getGasFeeTokensMock.mockResolvedValueOnce({ + gasFeeTokens: [GAS_FEE_TOKEN_MOCK], + isGasFeeSponsored: true, }); const { controller } = setupController(); @@ -3122,19 +2923,21 @@ describe('TransactionController', () => { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK, }, - { requireApproval: false, networkClientId: NETWORK_CLIENT_ID_MOCK }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, ); - expect(getBalanceChangesMock).toHaveBeenCalledTimes(0); - expect(controller.state.transactions[0].simulationData).toBeUndefined(); + await flushPromises(); + + expect(controller.state.transactions[0].isExternalSign).toBe(true); }); - }); - describe('updates gas fee tokens', () => { - it('by default', async () => { - getGasFeeTokensMock.mockResolvedValueOnce( - createGasFeeTokensResult([GAS_FEE_TOKEN_MOCK], true), - ); + it('does not set isExternalSign when transaction is not sponsored', async () => { + getGasFeeTokensMock.mockResolvedValueOnce({ + gasFeeTokens: [GAS_FEE_TOKEN_MOCK], + isGasFeeSponsored: false, + }); const { controller } = setupController(); @@ -3150,26 +2953,11 @@ describe('TransactionController', () => { await flushPromises(); - expect(controller.state.transactions[0]).toMatchObject({ - gasFeeTokens: [GAS_FEE_TOKEN_MOCK], - isGasFeeSponsoredAvailable: true, - }); + expect(controller.state.transactions[0].isExternalSign).toBeUndefined(); }); - it('with getSimulationConfig', async () => { - getGasFeeTokensMock.mockResolvedValueOnce( - createGasFeeTokensResult([GAS_FEE_TOKEN_MOCK]), - ); - - const getSimulationConfigMock: GetSimulationConfig = jest - .fn() - .mockResolvedValue({}); - - const { controller } = setupController({ - options: { - getSimulationConfig: getSimulationConfigMock, - }, - }); + it('sets isExternalSign to true immediately when isGasFeeSponsored is passed in options', async () => { + const { controller } = setupController(); await controller.addTransaction( { @@ -3178,27 +2966,18 @@ describe('TransactionController', () => { }, { networkClientId: NETWORK_CLIENT_ID_MOCK, + isGasFeeSponsored: true, }, ); - await flushPromises(); - - expect(getGasFeeTokensMock).toHaveBeenCalledTimes(1); - expect(getGasFeeTokensMock).toHaveBeenCalledWith( - expect.objectContaining({ - getSimulationConfig: getSimulationConfigMock, - }), - ); - - expect(controller.state.transactions[0].gasFeeTokens).toStrictEqual([ - GAS_FEE_TOKEN_MOCK, - ]); + expect(controller.state.transactions[0].isExternalSign).toBe(true); }); - it('unless approval not required', async () => { - getGasFeeTokensMock.mockResolvedValueOnce( - createGasFeeTokensResult([GAS_FEE_TOKEN_MOCK]), - ); + it('preserves isGasFeeSponsored and isExternalSign when passed in options even if simulation returns not sponsored', async () => { + getGasFeeTokensMock.mockResolvedValueOnce({ + gasFeeTokens: [], + isGasFeeSponsored: false, + }); const { controller } = setupController(); @@ -3207,11 +2986,16 @@ describe('TransactionController', () => { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK, }, - { requireApproval: false, networkClientId: NETWORK_CLIENT_ID_MOCK }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + isGasFeeSponsored: true, + }, ); - expect(getBalanceChangesMock).toHaveBeenCalledTimes(0); - expect(controller.state.transactions[0].gasFeeTokens).toBeUndefined(); + await flushPromises(); + + expect(controller.state.transactions[0].isGasFeeSponsored).toBe(true); + expect(controller.state.transactions[0].isExternalSign).toBe(true); }); }); @@ -3650,15 +3434,9 @@ describe('TransactionController', () => { expect(publishHook).toHaveBeenCalledTimes(1); }); - it('skips signing if shouldSign returns false and a publish hook is provided', async () => { - const { controller, mockTransactionApprovalRequest } = setupController({ - options: { - hooks: { - publish: async () => ({ transactionHash: TRANSACTION_HASH_MOCK }), - shouldSign: async () => ({ shouldSign: false }), - }, - }, - }); + it('skips signing if isExternalSign is true', async () => { + const { controller, mockTransactionApprovalRequest } = + setupController(); const { result, transactionMeta } = await controller.addTransaction( { @@ -3675,7 +3453,10 @@ describe('TransactionController', () => { mockTransactionApprovalRequest.approve({ value: { - txMeta: transactionMeta, + txMeta: { + ...transactionMeta, + isExternalSign: true, + }, }, }); @@ -3690,87 +3471,6 @@ describe('TransactionController', () => { ]); }); - it('defers to the publish hook without broadcasting an empty raw transaction when local signing is skipped', async () => { - const publishHook = jest.fn().mockResolvedValue({}); - const { controller, mockTransactionApprovalRequest } = setupController({ - options: { - hooks: { - publish: publishHook as NonNullable< - TransactionControllerOptions['hooks'] - >['publish'], - shouldSign: async () => ({ shouldSign: false }), - }, - }, - }); - - const { result, transactionMeta } = await controller.addTransaction( - { - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x0', - to: ACCOUNT_MOCK, - value: '0x0', - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - mockTransactionApprovalRequest.approve({ - value: { - txMeta: transactionMeta, - }, - }); - - await result; - - expect(controller.state.transactions).toMatchObject([ - expect.objectContaining({ - status: TransactionStatus.submitted, - }), - ]); - expect(publishHook).toHaveBeenCalledTimes(1); - expect(rpcRequestMock).not.toHaveBeenCalledWith( - expect.objectContaining({ method: 'eth_sendRawTransaction' }), - ); - }); - - it('fails without broadcasting an empty raw transaction when local signing is skipped without a publish hook', async () => { - const { controller, mockTransactionApprovalRequest } = setupController({ - options: { - hooks: { - shouldSign: async () => ({ shouldSign: false }), - }, - }, - }); - - const { result, transactionMeta } = await controller.addTransaction( - { - from: ACCOUNT_MOCK, - gas: '0x0', - gasPrice: '0x0', - to: ACCOUNT_MOCK, - value: '0x0', - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - mockTransactionApprovalRequest.approve({ - value: { - txMeta: transactionMeta, - }, - }); - - await expect(result).rejects.toThrow( - 'Cannot publish a transaction when there is no local signing and no publish hook', - ); - expect(rpcRequestMock).not.toHaveBeenCalledWith( - expect.objectContaining({ method: 'eth_sendRawTransaction' }), - ); - }); - describe('fails', () => { /** * Test template to assert adding and submitting a transaction fails. @@ -4092,35 +3792,6 @@ describe('TransactionController', () => { providerErrors.unauthorized({ data: { origin: expectedOrigin } }), ); }); - - it('reads internal accounts while validating an approved transaction', async () => { - const { controller, rootMessenger } = setupController({ - messengerOptions: { - addTransactionApprovalRequest: { - state: 'approved', - }, - }, - }); - - rootMessenger.unregisterActionHandler('AccountsController:getState'); - rootMessenger.registerActionHandler( - 'AccountsController:getState', - () => ({ - internalAccounts: { - accounts: { - [INTERNAL_ACCOUNT_MOCK.id]: INTERNAL_ACCOUNT_MOCK, - }, - }, - }), - ); - - const { result } = await controller.addTransaction( - { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK }, - { networkClientId: NETWORK_CLIENT_ID_MOCK }, - ); - - await result; - }); }); describe('updates submit history', () => { @@ -9271,9 +8942,10 @@ describe('TransactionController', () => { it('returns gas fee tokens', async () => { const { messenger } = setupController(); - getGasFeeTokensMock.mockResolvedValueOnce( - createGasFeeTokensResult([GAS_FEE_TOKEN_MOCK]), - ); + getGasFeeTokensMock.mockResolvedValueOnce({ + gasFeeTokens: [GAS_FEE_TOKEN_MOCK], + isGasFeeSponsored: false, + }); const result = await messenger.call( 'TransactionController:getGasFeeTokens', @@ -9289,12 +8961,13 @@ describe('TransactionController', () => { expect(result).toStrictEqual([GAS_FEE_TOKEN_MOCK]); }); - it('includes delegation address in request', async () => { + it('includes delegation address and isExternalSign in request', async () => { const { messenger } = setupController(); - getGasFeeTokensMock.mockResolvedValueOnce( - createGasFeeTokensResult([GAS_FEE_TOKEN_MOCK]), - ); + getGasFeeTokensMock.mockResolvedValueOnce({ + gasFeeTokens: [GAS_FEE_TOKEN_MOCK], + isGasFeeSponsored: false, + }); getDelegationAddressMock.mockResolvedValue(ACCOUNT_2_MOCK); @@ -9310,6 +8983,7 @@ describe('TransactionController', () => { expect.objectContaining({ transactionMeta: expect.objectContaining({ delegationAddress: ACCOUNT_2_MOCK, + isExternalSign: true, }), }), ); @@ -9366,40 +9040,6 @@ describe('TransactionController', () => { expect(approvedEventListener).not.toHaveBeenCalled(); }); - - it('publishes transactionApproved with a nonce after signing approval', async () => { - const { controller, messenger, mockTransactionApprovalRequest } = - setupController(); - - const approvedEventListener = jest.fn(); - - messenger.subscribe( - 'TransactionController:transactionApproved', - approvedEventListener, - ); - - const { result } = await controller.addTransaction( - { - from: ACCOUNT_MOCK, - gas: '0x21000', - gasPrice: '0x1', - to: ACCOUNT_MOCK, - value: '0x0', - }, - { - networkClientId: NETWORK_CLIENT_ID_MOCK, - }, - ); - - mockTransactionApprovalRequest.approve(); - - await result; - - expect(approvedEventListener).toHaveBeenCalledTimes(1); - expect( - approvedEventListener.mock.calls[0][0].transactionMeta.txParams.nonce, - ).toBeDefined(); - }); }); describe('TransactionController:estimateGasBatch', () => { diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 42652d846ba..778d5f56e36 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -124,8 +124,6 @@ import type { AfterAddHook, GasFeeEstimateLevel as GasFeeEstimateLevelType, TransactionBatchMeta, - IsGasSponsoredHook, - ShouldSignHook, BeforeSignHook, GetSimulationConfig, AddTransactionOptions, @@ -403,7 +401,7 @@ export type TransactionControllerOptions = { trace?: TraceCallback; /** The controller hooks. */ - hooks?: { + hooks: { /** Additional logic to execute after adding a transaction. */ afterAdd?: AfterAddHook; @@ -422,27 +420,15 @@ export type TransactionControllerOptions = { beforePublish?: (transactionMeta: TransactionMeta) => Promise; /** - * Preparation logic to execute before deciding whether to sign locally. - * Runs even when `shouldSign` returns false. + * Additional logic to execute before signing a transaction. */ beforeSign?: BeforeSignHook; - /** - * Additional logic to determine whether a transaction is sponsored. - */ - isSponsored?: IsGasSponsoredHook; - /** Alternate logic to publish a transaction. */ publish?: ( transactionMeta: TransactionMeta, ) => Promise<{ transactionHash: string }>; publishBatch?: PublishBatchHook; - - /** - * Policy logic to determine whether to reserve a nonce and sign locally. - * Use `beforeSign` for transaction preparation instead. - */ - shouldSign?: ShouldSignHook; }; }; @@ -746,10 +732,6 @@ export class TransactionController extends BaseController< transactionMeta: TransactionMeta, ) => Promise; - readonly #isSponsored: IsGasSponsoredHook; - - readonly #shouldSign: ShouldSignHook; - readonly #beforePublish: ( transactionMeta: TransactionMeta, ) => Promise; @@ -797,8 +779,6 @@ export class TransactionController extends BaseController< rawTx: string, ) => Promise<{ transactionHash?: string }>; - readonly #publishHookProvided: boolean; - readonly #publishBatchHook?: PublishBatchHook; readonly #signAbortCallbacks: Map void> = new Map(); @@ -853,13 +833,6 @@ export class TransactionController extends BaseController< /* istanbul ignore next */ hooks?.beforeCheckPendingTransaction ?? ((): Promise => Promise.resolve(true)); - this.#isSponsored = - hooks?.isSponsored ?? - ((): ReturnType => - Promise.resolve({ isSponsored: false })); - this.#shouldSign = - hooks?.shouldSign ?? - ((): ReturnType => Promise.resolve({ shouldSign: true })); this.#beforePublish = hooks?.beforePublish ?? ((): Promise => Promise.resolve(true)); this.#beforeSign = @@ -884,7 +857,6 @@ export class TransactionController extends BaseController< this.#isSwapsDisabled = disableSwaps ?? false; this.#isTimeoutEnabled = isTimeoutEnabled ?? ((): boolean => true); this.#publicKeyEIP7702 = publicKeyEIP7702; - this.#publishHookProvided = hooks?.publish !== undefined; this.#publish = hooks?.publish ?? ((): Promise<{ transactionHash?: string }> => @@ -1218,8 +1190,9 @@ export class TransactionController extends BaseController< id: random(), isGasFeeTokenIgnoredIfBalance, isGasFeeIncluded, + isGasFeeSponsored, + ...(isGasFeeSponsored ? { isExternalSign: true } : {}), // To avoid the property to be set as undefined. - ...(isGasFeeSponsored === undefined ? {} : { isGasFeeSponsored }), ...(excludeNativeTokenForFee === undefined ? {} : { excludeNativeTokenForFee }), @@ -3139,6 +3112,20 @@ export class TransactionController extends BaseController< clearApprovingTransactionId = (): boolean => this.#approvingTransactionIds.delete(transactionId); + const { networkClientId } = transactionMeta; + + const [nonce, releaseNonce] = await getNextNonce( + transactionMeta, + (address: string) => + this.#multichainTrackingHelper.getNonceLock( + address, + transactionMeta.networkClientId, + ), + ); + + clearNonceLock = releaseNonce; + + // eslint-disable-next-line require-atomic-updates transactionMeta = this.#updateTransactionInternal( { transactionId, @@ -3150,6 +3137,7 @@ export class TransactionController extends BaseController< draftTxMeta.status = TransactionStatus.approved; draftTxMeta.txParams.chainId = chainId; draftTxMeta.txParams.gasLimit = gas; + draftTxMeta.txParams.nonce = nonce; if (!type && isEIP1559Transaction(txParams)) { draftTxMeta.txParams.type = TransactionEnvelopeType.feeMarket; @@ -3157,56 +3145,15 @@ export class TransactionController extends BaseController< }, ); - // eslint-disable-next-line require-atomic-updates - transactionMeta = await this.#applyBeforeSignHook(transactionMeta); - - // eslint-disable-next-line require-atomic-updates - transactionMeta = - await this.#prepareTransactionBeforeApproval(transactionMeta); - - const { networkClientId } = transactionMeta; - - const { isSponsored } = await this.#isSponsored({ transactionMeta }); - const shouldSign = isSponsored - ? false - : (await this.#shouldSign({ transactionMeta })).shouldSign; - - let rawTx: string | undefined; - - if (shouldSign) { - const [nonce, releaseNonce] = await getNextNonce( - transactionMeta, - (address: string) => - this.#multichainTrackingHelper.getNonceLock( - address, - transactionMeta.networkClientId, - ), - ); - - clearNonceLock = releaseNonce; - - // eslint-disable-next-line require-atomic-updates - transactionMeta = this.#updateTransactionInternal( - { - transactionId, - }, - (draftTxMeta) => { - draftTxMeta.txParams.nonce = nonce; - }, - ); - } - this.#onTransactionStatusChange(transactionMeta); - if (shouldSign) { - rawTx = await this.#trace( - { name: 'Sign', parentContext: traceContext }, - () => this.#signTransaction(transactionMeta), - ); + const rawTx = await this.#trace( + { name: 'Sign', parentContext: traceContext }, + () => this.#signTransaction(transactionMeta), + ); - // eslint-disable-next-line require-atomic-updates - transactionMeta = this.#getTransactionOrThrow(transactionId); - } + // eslint-disable-next-line require-atomic-updates + transactionMeta = this.#getTransactionOrThrow(transactionId); if (!(await this.#beforePublish(transactionMeta))) { log('Skipping publishing transaction based on hook'); @@ -3217,16 +3164,10 @@ export class TransactionController extends BaseController< return ApprovalState.SkippedViaBeforePublishHook; } - if (!rawTx && shouldSign) { + if (!rawTx && !transactionMeta.isExternalSign) { return ApprovalState.NotApproved; } - if (!shouldSign && !publishHookOverride && !this.#publishHookProvided) { - throw new Error( - 'Cannot publish a transaction when there is no local signing and no publish hook', - ); - } - let preTxBalance: string | undefined; const shouldUpdatePreTxBalance = transactionMeta.type === TransactionType.swap; @@ -3730,9 +3671,10 @@ export class TransactionController extends BaseController< ); } - async #applyBeforeSignHook( - transactionMeta: TransactionMeta, - ): Promise { + async #signTransaction( + originalTransactionMeta: TransactionMeta, + ): Promise { + let transactionMeta = originalTransactionMeta; const { id: transactionId } = transactionMeta; log('Calling before sign hook', transactionMeta); @@ -3749,54 +3691,28 @@ export class TransactionController extends BaseController< log('Updated transaction after before sign hook'); } - return this.#getTransactionOrThrow(transactionId); - } + transactionMeta = this.#getTransactionOrThrow(transactionId); - async #prepareTransactionBeforeApproval( - transactionMeta: TransactionMeta, - ): Promise { - const { id: transactionId, networkClientId } = transactionMeta; - let preparedTransactionMeta = transactionMeta; - let gasFeeTokens = transactionMeta.gasFeeTokens ?? []; - const shouldRefreshGasFeeTokens = - this.#isSimulationEnabled(transactionMeta) || - (Boolean(transactionMeta.selectedGasFeeToken) && - (transactionMeta.isGasFeeTokenIgnoredIfBalance === true || - transactionMeta.excludeNativeTokenForFee === true)); - - if (shouldRefreshGasFeeTokens) { - this.#simulationRequestTokens.delete(transactionId); - const gasFeeTokensResult = await this.#getGasFeeTokens(transactionMeta); - gasFeeTokens = gasFeeTokensResult.gasFeeTokens; - - preparedTransactionMeta = this.#updateTransactionInternal( - { transactionId }, - (draftTxMeta) => { - draftTxMeta.gasFeeTokens = gasFeeTokens; - draftTxMeta.isGasFeeSponsoredAvailable = - gasFeeTokensResult.isGasFeeSponsoredAvailable; - }, - ); - } + const { networkClientId } = transactionMeta; await checkGasFeeTokenBeforePublish({ messenger: this.messenger, networkClientId, - fetchGasFeeTokens: async () => gasFeeTokens, - transaction: preparedTransactionMeta, + fetchGasFeeTokens: async (tx) => + (await this.#getGasFeeTokens(tx)).gasFeeTokens, + transaction: transactionMeta, updateTransaction: (txId, fn) => this.#updateTransactionInternal({ transactionId: txId }, fn), }); - return this.#getTransactionOrThrow(transactionId); - } + transactionMeta = this.#getTransactionOrThrow(transactionId); + const { chainId, isExternalSign, txParams } = transactionMeta; + + if (isExternalSign) { + log('Skipping sign as signed externally'); + return undefined; + } - async #signTransaction( - originalTransactionMeta: TransactionMeta, - ): Promise { - let transactionMeta = originalTransactionMeta; - const { id: transactionId } = transactionMeta; - const { chainId, txParams } = transactionMeta; const { authorizationList, from } = txParams; const signedAuthorizationList = await signAuthorizationList({ @@ -4196,7 +4112,7 @@ export class TransactionController extends BaseController< }; let gasUsed: Hex | undefined; let gasFeeTokens: GasFeeToken[] = []; - let isGasFeeSponsoredAvailable: boolean | undefined; + let isGasFeeSponsored = false; let simulationRevert: Revert | undefined; const simulationRequestToken = Symbol(transactionId); @@ -4243,8 +4159,11 @@ export class TransactionController extends BaseController< } if (isSimulationEnabled) { - ({ gasFeeTokens, isGasFeeSponsoredAvailable } = - await this.#getGasFeeTokens(transactionMeta)); + const gasFeeTokensResponse = + await this.#getGasFeeTokens(transactionMeta); + + gasFeeTokens = gasFeeTokensResponse?.gasFeeTokens ?? []; + isGasFeeSponsored = gasFeeTokensResponse?.isGasFeeSponsored ?? false; } if ( @@ -4275,8 +4194,11 @@ export class TransactionController extends BaseController< }, (txMeta) => { txMeta.gasFeeTokens = gasFeeTokens; - if (isGasFeeSponsoredAvailable !== undefined) { - txMeta.isGasFeeSponsoredAvailable = isGasFeeSponsoredAvailable; + txMeta.isGasFeeSponsored = + txMeta.isGasFeeSponsored ?? isGasFeeSponsored; + + if (txMeta.isGasFeeSponsored) { + txMeta.isExternalSign = true; } if (!this.#isBalanceChangesSkipped(txMeta)) { @@ -4556,14 +4478,12 @@ export class TransactionController extends BaseController< ({ transactionHash } = await publishHook(transactionMeta, signedTx)); - if (signedTx !== '0x') { - // eslint-disable-next-line require-atomic-updates - transactionHash ??= await this.#publishTransaction({ - ...transactionMeta, - networkClientId, - rawTx: signedTx, - }); - } + // eslint-disable-next-line require-atomic-updates + transactionHash ??= await this.#publishTransaction({ + ...transactionMeta, + networkClientId, + rawTx: signedTx, + }); }, ); @@ -4572,9 +4492,10 @@ export class TransactionController extends BaseController< return { transactionHash }; } - async #getGasFeeTokens( - transaction: TransactionMeta, - ): ReturnType { + async #getGasFeeTokens(transaction: TransactionMeta): Promise<{ + gasFeeTokens: GasFeeToken[]; + isGasFeeSponsored: boolean; + }> { const { chainId } = transaction; return await getGasFeeTokens({ @@ -4610,6 +4531,7 @@ export class TransactionController extends BaseController< const transaction = { chainId, delegationAddress, + isExternalSign: true, txParams: { data, from, @@ -4618,6 +4540,8 @@ export class TransactionController extends BaseController< }, } as TransactionMeta; - return (await this.#getGasFeeTokens(transaction)).gasFeeTokens; + const result = await this.#getGasFeeTokens(transaction); + + return result.gasFeeTokens; } } diff --git a/packages/transaction-controller/src/index.ts b/packages/transaction-controller/src/index.ts index 09a8c6218fa..b7b17038a97 100644 --- a/packages/transaction-controller/src/index.ts +++ b/packages/transaction-controller/src/index.ts @@ -71,8 +71,6 @@ export type { BatchTransaction, BatchTransactionParams, BeforeSignHook, - IsGasSponsoredHook, - ShouldSignHook, DappSuggestedGasFees, DefaultGasEstimates, FeeMarketEIP1559Values, diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index 145248277bd..3b1297dd929 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -224,13 +224,8 @@ export type TransactionMeta = { */ isFirstTimeInteraction?: boolean; - /** Whether gas fee sponsorship is available for the transaction. */ - isGasFeeSponsoredAvailable?: boolean; - /** * Whether the transaction is sponsored meaning the user does not pay the gas fee. - * - * @deprecated No longer used by the transaction lifecycle. Use the `isSponsored` hook instead. */ isGasFeeSponsored?: boolean; @@ -272,8 +267,7 @@ export type TransactionMeta = { /** * Whether the transaction is signed externally. - * - * @deprecated No longer used by the transaction lifecycle. Use the `shouldSign` hook instead. + * No signing will be performed in the client and the `nonce` will be `undefined`. */ isExternalSign?: boolean; @@ -1883,11 +1877,7 @@ export type TransactionBatchRequest = { /** Whether MetaMask will be compensated for the gas fee by the transaction. */ isGasFeeIncluded?: boolean; - /** - * Whether MetaMask will sponsor the gas fee for the transaction. - * - * @deprecated No longer used by the transaction lifecycle. Use the `isSponsored` hook instead. - */ + /** Whether MetaMask will sponsor the gas fee for the transaction. */ isGasFeeSponsored?: boolean; /** ID of the network client to submit the transaction. */ @@ -2146,9 +2136,8 @@ export type AfterAddHook = (request: { }>; /** - * Preparation logic to execute before deciding whether to sign locally. - * Runs even when {@link ShouldSignHook} returns false and can optionally update - * the transaction by returning the `updateTransaction` callback. + * Custom logic to be executed before a transaction is signed. + * Can optionally update the transaction by returning the `updateTransaction` callback. */ export type BeforeSignHook = (request: { transactionMeta: TransactionMeta; @@ -2159,22 +2148,6 @@ export type BeforeSignHook = (request: { | undefined >; -/** - * Custom logic to determine whether a transaction should be treated as sponsored. - */ -export type IsGasSponsoredHook = (request: { - transactionMeta: TransactionMeta; -}) => Promise<{ isSponsored: boolean }>; - -/** - * Policy logic to determine whether to reserve a nonce and sign locally for a - * non-sponsored transaction. Use {@link BeforeSignHook} for transaction - * preparation instead. - */ -export type ShouldSignHook = (request: { - transactionMeta: TransactionMeta; -}) => Promise<{ shouldSign: boolean }>; - /** * The total fiat values of the transaction, to support client metrics. */ @@ -2271,11 +2244,7 @@ export type AddTransactionOptions = { /** Whether MetaMask will be compensated for the gas fee by the transaction. */ isGasFeeIncluded?: boolean; - /** - * Whether MetaMask will sponsor the gas fee for the transaction. - * - * @deprecated No longer used by the transaction lifecycle. Use the `isSponsored` hook instead. - */ + /** Whether MetaMask will sponsor the gas fee for the transaction. */ isGasFeeSponsored?: boolean; /** When set to `true` and if gasFeeToken is set, use gasFeeToken regardless of user native balance. */ diff --git a/packages/transaction-controller/src/utils/gas-fee-tokens.test.ts b/packages/transaction-controller/src/utils/gas-fee-tokens.test.ts index 0a2a832aad4..ea9db48a3dc 100644 --- a/packages/transaction-controller/src/utils/gas-fee-tokens.test.ts +++ b/packages/transaction-controller/src/utils/gas-fee-tokens.test.ts @@ -151,7 +151,7 @@ describe('Gas Fee Tokens Utils', () => { tokenAddress: TOKEN_ADDRESS_2_MOCK, }, ], - isGasFeeSponsoredAvailable: true, + isGasFeeSponsored: true, }); }); @@ -229,7 +229,7 @@ describe('Gas Fee Tokens Utils', () => { tokenAddress: TOKEN_ADDRESS_1_MOCK, }, ], - isGasFeeSponsoredAvailable: true, + isGasFeeSponsored: true, }); }); @@ -242,7 +242,7 @@ describe('Gas Fee Tokens Utils', () => { expect(result).toStrictEqual({ gasFeeTokens: [], - isGasFeeSponsoredAvailable: false, + isGasFeeSponsored: false, }); }); @@ -374,44 +374,6 @@ describe('Gas Fee Tokens Utils', () => { ); }); - it('returns empty gas fee tokens if the EIP-7702 public key is not provided', async () => { - const request = cloneDeep(REQUEST_MOCK); - request.publicKeyEIP7702 = undefined; - - doesChainSupportEIP7702Mock.mockReturnValueOnce(true); - simulateTransactionsMock.mockResolvedValueOnce({ - transactions: [], - sponsorship: { - isSponsored: false, - error: null, - }, - }); - - expect(await getGasFeeTokens(request)).toStrictEqual({ - gasFeeTokens: [], - isGasFeeSponsoredAvailable: false, - }); - }); - - it('returns empty gas fee tokens if the upgrade contract address cannot be resolved', async () => { - const request = cloneDeep(REQUEST_MOCK); - - doesChainSupportEIP7702Mock.mockReturnValueOnce(true); - getEIP7702UpgradeContractAddressMock.mockReturnValueOnce(undefined); - simulateTransactionsMock.mockResolvedValueOnce({ - transactions: [], - sponsorship: { - isSponsored: false, - error: null, - }, - }); - - expect(await getGasFeeTokens(request)).toStrictEqual({ - gasFeeTokens: [], - isGasFeeSponsoredAvailable: false, - }); - }); - it('forwards simulation config', async () => { const getSimulationConfigMock: GetSimulationConfig = jest.fn(); @@ -449,6 +411,7 @@ describe('Gas Fee Tokens Utils', () => { request.transaction.isGasFeeTokenIgnoredIfBalance = true; request.transaction.selectedGasFeeToken = TOKEN_ADDRESS_1_MOCK; request.transaction.gasFeeTokens = []; + request.transaction.isExternalSign = true; jest.mocked(request.fetchGasFeeTokens).mockResolvedValueOnce([]); @@ -460,6 +423,7 @@ describe('Gas Fee Tokens Utils', () => { .mocked(request.updateTransaction) .mock.calls[0][1](request.transaction); + expect(request.transaction.isExternalSign).toBe(false); expect(request.transaction.gasFeeTokens).toStrictEqual([]); }); @@ -478,10 +442,33 @@ describe('Gas Fee Tokens Utils', () => { expect(request.fetchGasFeeTokens).toHaveBeenCalledTimes(1); expect(request.fetchGasFeeTokens).toHaveBeenCalledWith( - request.transaction, + expect.objectContaining({ + isExternalSign: true, + }), ); }); + it('sets external sign to true if gas fee token found', async () => { + request.transaction.isGasFeeTokenIgnoredIfBalance = true; + request.transaction.selectedGasFeeToken = TOKEN_ADDRESS_1_MOCK; + request.transaction.gasFeeTokens = []; + request.transaction.isExternalSign = false; + + jest.mocked(request.fetchGasFeeTokens).mockResolvedValueOnce([ + { + tokenAddress: TOKEN_ADDRESS_1_MOCK, + } as GasFeeToken, + ]); + + await checkGasFeeTokenBeforePublish(request); + + jest + .mocked(request.updateTransaction) + .mock.calls[0][1](request.transaction); + + expect(request.transaction.isExternalSign).toBe(true); + }); + it('removes nonce if gas fee token found', async () => { request.transaction.isGasFeeTokenIgnoredIfBalance = true; request.transaction.selectedGasFeeToken = TOKEN_ADDRESS_1_MOCK; @@ -503,11 +490,12 @@ describe('Gas Fee Tokens Utils', () => { expect(request.transaction.txParams.nonce).toBeUndefined(); }); - it('skips the native balance check when native token is excluded for fees', async () => { + it('sets external sign when native token is excluded for fees', async () => { request.transaction.excludeNativeTokenForFee = true; request.transaction.isGasFeeTokenIgnoredIfBalance = false; request.transaction.selectedGasFeeToken = TOKEN_ADDRESS_1_MOCK; request.transaction.gasFeeTokens = []; + request.transaction.isExternalSign = false; request.transaction.txParams.nonce = '0x1'; jest.mocked(request.fetchGasFeeTokens).mockResolvedValueOnce([ @@ -524,14 +512,18 @@ describe('Gas Fee Tokens Utils', () => { expect(isNativeBalanceSufficientForGasMock).not.toHaveBeenCalled(); expect(request.fetchGasFeeTokens).toHaveBeenCalledWith( - request.transaction, + expect.objectContaining({ + isExternalSign: true, + }), ); + expect(request.transaction.isExternalSign).toBe(true); expect(request.transaction.txParams.nonce).toBeUndefined(); }); it('removes selected gas fee token if native balance sufficient', async () => { request.transaction.isGasFeeTokenIgnoredIfBalance = true; request.transaction.selectedGasFeeToken = TOKEN_ADDRESS_1_MOCK; + request.transaction.isExternalSign = true; isNativeBalanceSufficientForGasMock.mockResolvedValueOnce(true); @@ -542,6 +534,7 @@ describe('Gas Fee Tokens Utils', () => { .mock.calls[0][1](request.transaction); expect(request.transaction.selectedGasFeeToken).toBeUndefined(); + expect(request.transaction.isExternalSign).toBe(false); }); it('does nothing if no selected gas fee token', async () => { diff --git a/packages/transaction-controller/src/utils/gas-fee-tokens.ts b/packages/transaction-controller/src/utils/gas-fee-tokens.ts index 8b4ce10c341..849cf607098 100644 --- a/packages/transaction-controller/src/utils/gas-fee-tokens.ts +++ b/packages/transaction-controller/src/utils/gas-fee-tokens.ts @@ -44,7 +44,7 @@ export type GetGasFeeTokensRequest = { * @param request.publicKeyEIP7702 - Public key to validate EIP-7702 contract signatures. * @param request.transactionMeta - The transaction metadata. * @param request.getSimulationConfig - Optional transaction simulation parameters. - * @returns The available gas fee tokens and whether gas fee sponsorship is available. + * @returns An array of gas fee tokens. */ export async function getGasFeeTokens({ chainId, @@ -55,7 +55,7 @@ export async function getGasFeeTokens({ getSimulationConfig, }: GetGasFeeTokensRequest): Promise<{ gasFeeTokens: GasFeeToken[]; - isGasFeeSponsoredAvailable: boolean; + isGasFeeSponsored: boolean; }> { const { delegationAddress, txParams } = transactionMeta; const { authorizationList: authorizationListRequest } = txParams; @@ -116,7 +116,7 @@ export async function getGasFeeTokens({ return result; } catch (error) { log('Failed to gas fee tokens', error); - return { gasFeeTokens: [], isGasFeeSponsoredAvailable: false }; + return { gasFeeTokens: [], isGasFeeSponsored: false }; } } @@ -174,6 +174,7 @@ export async function checkGasFeeTokenBeforePublish({ ); updateTransaction(transaction.id, (tx) => { + tx.isExternalSign = false; tx.selectedGasFeeToken = undefined; }); @@ -181,7 +182,10 @@ export async function checkGasFeeTokenBeforePublish({ } } - const gasFeeTokens = await fetchGasFeeTokens(transaction); + const gasFeeTokens = await fetchGasFeeTokens({ + ...transaction, + isExternalSign: true, + }); const isSelectedGasFeeTokenAvailable = gasFeeTokens?.some( (token) => @@ -191,6 +195,7 @@ export async function checkGasFeeTokenBeforePublish({ if (!isSelectedGasFeeTokenAvailable) { updateTransaction(transaction.id, (tx) => { tx.gasFeeTokens = gasFeeTokens; + tx.isExternalSign = false; }); throw new Error('Gas fee token not found and insufficient native balance'); @@ -198,6 +203,7 @@ export async function checkGasFeeTokenBeforePublish({ updateTransaction(transaction.id, (tx) => { tx.gasFeeTokens = gasFeeTokens; + tx.isExternalSign = true; tx.txParams.nonce = undefined; }); @@ -209,15 +215,17 @@ export async function checkGasFeeTokenBeforePublish({ * Extract gas fee tokens from a simulation response. * * @param response - The simulation response. - * @returns The available gas fee tokens and whether gas fee sponsorship is available. + * @returns gasFeeTokens: An array of gas fee tokens. isGasFeeSponsored: Whether the transaction is sponsored */ function parseGasFeeTokens(response: SimulationResponse): { gasFeeTokens: GasFeeToken[]; - isGasFeeSponsoredAvailable: boolean; + isGasFeeSponsored: boolean; } { const feeLevel = response.transactions?.[0] ?.fees?.[0] as Required['fees'][0]; + const isGasFeeSponsored = response.sponsorship?.isSponsored ?? false; + const tokenFees = feeLevel?.tokenFees ?? []; return { @@ -235,7 +243,7 @@ function parseGasFeeTokens(response: SimulationResponse): { symbol: tokenFee.token.symbol, tokenAddress: tokenFee.token.address, })), - isGasFeeSponsoredAvailable: response.sponsorship?.isSponsored ?? false, + isGasFeeSponsored, }; } diff --git a/packages/transaction-controller/src/utils/nonce.test.ts b/packages/transaction-controller/src/utils/nonce.test.ts index dfec8b02fb9..db8fd3f37b0 100644 --- a/packages/transaction-controller/src/utils/nonce.test.ts +++ b/packages/transaction-controller/src/utils/nonce.test.ts @@ -104,6 +104,21 @@ describe('nonce', () => { expect(releaseLock).toHaveBeenCalledTimes(1); }); + + it('returns undefined if transaction is signed externally', async () => { + const transactionMeta = { + ...TRANSACTION_META_MOCK, + isExternalSign: true, + }; + + const [nonce, releaseLock] = await getNextNonce( + transactionMeta, + jest.fn(), + ); + + expect(nonce).toBeUndefined(); + expect(releaseLock).toBeUndefined(); + }); }); describe('getAndFormatTransactionsForNonceTracker', () => { diff --git a/packages/transaction-controller/src/utils/nonce.ts b/packages/transaction-controller/src/utils/nonce.ts index 06d04acdbf4..550f434adcf 100644 --- a/packages/transaction-controller/src/utils/nonce.ts +++ b/packages/transaction-controller/src/utils/nonce.ts @@ -23,9 +23,15 @@ export async function getNextNonce( ): Promise<[string | undefined, (() => void) | undefined]> { const { customNonceValue, + isExternalSign, txParams: { from, nonce: existingNonce }, } = txMeta; + if (isExternalSign) { + log('Skipping nonce as signed externally'); + return [undefined, undefined]; + } + const customNonce = customNonceValue ? toHex(customNonceValue) : undefined; if (customNonce) {