From 0d0b280ee1adf37699ddb0518f1e7ed7b0e04998 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Wed, 9 Sep 2026 15:17:04 +0100 Subject: [PATCH 1/7] fix(transaction-controller): restore gas fee preflight on approval --- packages/transaction-controller/CHANGELOG.md | 6 + .../src/TransactionController.test.ts | 124 +++++++++++++ .../src/TransactionController.ts | 164 ++++++++++++++---- packages/transaction-controller/src/index.ts | 2 + packages/transaction-controller/src/types.ts | 15 ++ 5 files changed, 276 insertions(+), 35 deletions(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 6bd369e5234..a560f87f90d 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `uuid` from `^8.3.2` to `^9.0.1` ([#10117](https://github.com/MetaMask/core/pull/10117)) - Bump `@metamask/core-backend` from `^10.0.0` to `^10.0.1` ([#10166](https://github.com/MetaMask/core/pull/10166)) +- Add approval-time sponsorship/signing hooks to `TransactionController` and keep gas-fee-token preflight in the approval flow ([#10109](https://github.com/MetaMask/core/pull/10109)) ## [70.0.0] @@ -33,6 +34,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [69.8.1] +### Changed + +- Bump `@metamask/core-backend` from `^9.0.0` to `^9.1.1` ([#10138](https://github.com/MetaMask/core/pull/10138), [#10139](https://github.com/MetaMask/core/pull/10139)) +- Bump `@metamask/remote-feature-flag-controller` from `^6.1.0` to `^6.1.1` ([#10129](https://github.com/MetaMask/core/pull/10129)) + ### Fixed - Harden gas fee token preflight by not treating pending gas estimates as zero-cost native gas, and by resetting `isExternalSign` when preflight validation fails ([#10071](https://github.com/MetaMask/core/pull/10071)) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index b32d8f575f5..38d5ec89ca6 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -2409,6 +2409,130 @@ describe('TransactionController', () => { }); }); + describe('with sponsored approval hooks', () => { + it('calls isSponsored hook before reserving a nonce', async () => { + const callOrder: string[] = []; + + const isSponsoredHook = jest.fn().mockImplementation(async () => { + callOrder.push('isSponsored'); + expect(getNonceLockSpy).not.toHaveBeenCalled(); + return false; + }); + + const shouldSignHook = jest.fn().mockImplementation(async () => { + callOrder.push('shouldSign'); + expect(getNonceLockSpy).not.toHaveBeenCalled(); + return true; + }); + + getNonceLockSpy.mockImplementation(async () => { + callOrder.push('getNonceLock'); + return { + nextNonce: NONCE_MOCK, + releaseLock: () => Promise.resolve(), + }; + }); + + const { controller } = setupController({ + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, + options: { + hooks: { + isSponsored: isSponsoredHook, + shouldSign: shouldSignHook, + }, + }, + }); + + 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']); + }); + + it('skips nonce reservation when shouldSign resolves false', async () => { + const isSponsoredHook = jest.fn().mockResolvedValue(false); + const shouldSignHook = jest.fn().mockResolvedValue(false); + + const { controller } = setupController({ + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, + options: { + hooks: { + isSponsored: isSponsoredHook, + shouldSign: shouldSignHook, + }, + }, + }); + + 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(getNonceLockSpy).not.toHaveBeenCalled(); + }); + + 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, + shouldSign: jest.fn().mockResolvedValue(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({}); diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 778d5f56e36..098d22dc760 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -124,6 +124,8 @@ import type { AfterAddHook, GasFeeEstimateLevel as GasFeeEstimateLevelType, TransactionBatchMeta, + IsSponsoredHook, + ShouldSignHook, BeforeSignHook, GetSimulationConfig, AddTransactionOptions, @@ -413,6 +415,16 @@ export type TransactionControllerOptions = { transactionMeta: TransactionMeta, ) => Promise; + /** + * Additional logic to determine whether a transaction is sponsored. + */ + isSponsored?: IsSponsoredHook; + + /** + * Additional logic to determine whether a transaction should be signed locally. + */ + shouldSign?: ShouldSignHook; + /** * Additional logic to execute before publishing a transaction. * Return false to prevent the broadcast of the transaction. @@ -732,6 +744,10 @@ export class TransactionController extends BaseController< transactionMeta: TransactionMeta, ) => Promise; + readonly #isSponsored: IsSponsoredHook; + + readonly #shouldSign: ShouldSignHook; + readonly #beforePublish: ( transactionMeta: TransactionMeta, ) => Promise; @@ -833,6 +849,19 @@ export class TransactionController extends BaseController< /* istanbul ignore next */ hooks?.beforeCheckPendingTransaction ?? ((): Promise => Promise.resolve(true)); + this.#isSponsored = + hooks?.isSponsored ?? + (async ({ transactionMeta }: { transactionMeta: TransactionMeta }) => + Boolean(transactionMeta.isGasFeeSponsored)); + this.#shouldSign = + hooks?.shouldSign ?? + (async ({ + transactionMeta, + isSponsored, + }: { + transactionMeta: TransactionMeta; + isSponsored: boolean; + }) => !Boolean(transactionMeta.isExternalSign)); this.#beforePublish = hooks?.beforePublish ?? ((): Promise => Promise.resolve(true)); this.#beforeSign = @@ -3112,19 +3141,6 @@ 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( { @@ -3137,7 +3153,6 @@ 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; @@ -3147,14 +3162,78 @@ export class TransactionController extends BaseController< this.#onTransactionStatusChange(transactionMeta); - const rawTx = await this.#trace( - { name: 'Sign', parentContext: traceContext }, - () => this.#signTransaction(transactionMeta), - ); + transactionMeta = await this.#applyBeforeSignHook(transactionMeta); + + const { networkClientId } = transactionMeta; + + await checkGasFeeTokenBeforePublish({ + messenger: this.messenger, + networkClientId, + fetchGasFeeTokens: async (tx) => + (await this.#getGasFeeTokens(tx)).gasFeeTokens, + transaction: transactionMeta, + updateTransaction: (txId, fn) => + this.#updateTransactionInternal({ transactionId: txId }, fn), + }); - // eslint-disable-next-line require-atomic-updates transactionMeta = this.#getTransactionOrThrow(transactionId); + const isSponsored = await this.#isSponsored({ transactionMeta }); + const shouldSign = await this.#shouldSign({ + transactionMeta, + isSponsored, + }); + + // eslint-disable-next-line require-atomic-updates + transactionMeta = this.#updateTransactionInternal( + { + transactionId, + }, + (draftTxMeta) => { + draftTxMeta.isGasFeeSponsored = isSponsored; + draftTxMeta.isExternalSign = !shouldSign; + + if (!shouldSign) { + draftTxMeta.txParams.nonce = undefined; + } + }, + ); + + 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); + + rawTx = await this.#trace( + { name: 'Sign', parentContext: traceContext }, + () => this.#signTransaction(transactionMeta, true, true), + ); + + // eslint-disable-next-line require-atomic-updates + transactionMeta = this.#getTransactionOrThrow(transactionId); + } + if (!(await this.#beforePublish(transactionMeta))) { log('Skipping publishing transaction based on hook'); this.messenger.publish( @@ -3671,10 +3750,9 @@ export class TransactionController extends BaseController< ); } - async #signTransaction( - originalTransactionMeta: TransactionMeta, - ): Promise { - let transactionMeta = originalTransactionMeta; + async #applyBeforeSignHook( + transactionMeta: TransactionMeta, + ): Promise { const { id: transactionId } = transactionMeta; log('Calling before sign hook', transactionMeta); @@ -3691,21 +3769,37 @@ export class TransactionController extends BaseController< log('Updated transaction after before sign hook'); } - transactionMeta = this.#getTransactionOrThrow(transactionId); + return this.#getTransactionOrThrow(transactionId); + } - const { networkClientId } = transactionMeta; + async #signTransaction( + originalTransactionMeta: TransactionMeta, + skipGasFeeTokenCheck = false, + skipBeforeSign = false, + ): Promise { + let transactionMeta = originalTransactionMeta; + const { id: transactionId } = transactionMeta; - await checkGasFeeTokenBeforePublish({ - messenger: this.messenger, - networkClientId, - fetchGasFeeTokens: async (tx) => - (await this.#getGasFeeTokens(tx)).gasFeeTokens, - transaction: transactionMeta, - updateTransaction: (txId, fn) => - this.#updateTransactionInternal({ transactionId: txId }, fn), - }); + if (!skipBeforeSign) { + transactionMeta = await this.#applyBeforeSignHook(transactionMeta); + } + + if (!skipGasFeeTokenCheck) { + const { networkClientId } = transactionMeta; + + await checkGasFeeTokenBeforePublish({ + messenger: this.messenger, + networkClientId, + fetchGasFeeTokens: async (tx) => + (await this.#getGasFeeTokens(tx)).gasFeeTokens, + transaction: transactionMeta, + updateTransaction: (txId, fn) => + this.#updateTransactionInternal({ transactionId: txId }, fn), + }); + + transactionMeta = this.#getTransactionOrThrow(transactionId); + } - transactionMeta = this.#getTransactionOrThrow(transactionId); const { chainId, isExternalSign, txParams } = transactionMeta; if (isExternalSign) { diff --git a/packages/transaction-controller/src/index.ts b/packages/transaction-controller/src/index.ts index b7b17038a97..4905020bd28 100644 --- a/packages/transaction-controller/src/index.ts +++ b/packages/transaction-controller/src/index.ts @@ -71,6 +71,8 @@ export type { BatchTransaction, BatchTransactionParams, BeforeSignHook, + IsSponsoredHook, + ShouldSignHook, DappSuggestedGasFees, DefaultGasEstimates, FeeMarketEIP1559Values, diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index 83b19561560..bd9b9e4cafa 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -2130,6 +2130,21 @@ export type AfterAddHook = (request: { updateTransaction?: (transaction: TransactionMeta) => void; }>; +/** + * Custom logic to determine whether a transaction should be treated as sponsored. + */ +export type IsSponsoredHook = (request: { + transactionMeta: TransactionMeta; +}) => Promise; + +/** + * Custom logic to determine whether a transaction should be signed locally. + */ +export type ShouldSignHook = (request: { + transactionMeta: TransactionMeta; + isSponsored: boolean; +}) => Promise; + /** * Custom logic to be executed before a transaction is signed. * Can optionally update the transaction by returning the `updateTransaction` callback. From 103e6eeae39c2fe6a726c7e4b4f5037b20a64ef5 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 10 Sep 2026 10:49:52 +0100 Subject: [PATCH 2/7] test(transaction-controller): cover approval edge cases --- .../src/TransactionController.test.ts | 61 +++++++++++++++++++ .../src/api/simulation-api.test.ts | 21 +++++++ .../src/utils/gas-fee-tokens.test.ts | 38 ++++++++++++ .../src/utils/prepare.test.ts | 1 + .../src/utils/prepare.ts | 12 +--- 5 files changed, 124 insertions(+), 9 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 38d5ec89ca6..bc79c99d474 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -964,6 +964,41 @@ 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: { @@ -3916,6 +3951,32 @@ 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', () => { diff --git a/packages/transaction-controller/src/api/simulation-api.test.ts b/packages/transaction-controller/src/api/simulation-api.test.ts index 919af70645a..4336623cdac 100644 --- a/packages/transaction-controller/src/api/simulation-api.test.ts +++ b/packages/transaction-controller/src/api/simulation-api.test.ts @@ -194,5 +194,26 @@ describe('Simulation API Utils', () => { code: expect.any(String), }); }); + + it('creates overrides when they are not already present', async () => { + const request = cloneDeep(REQUEST_MOCK); + request.overrides = undefined; + request.transactions[0].to = + DELEGATION_MANAGER_ADDRESSES[0].toUpperCase() as Hex; + + await simulateTransactions(CHAIN_ID_MOCK, request); + + expect(fetchMock).toHaveBeenCalledTimes(2); + + const requestBody = JSON.parse( + fetchMock.mock.calls[1][1]?.body as string, + ); + + expect(requestBody.params[0].overrides).toStrictEqual({ + [DELEGATION_MANAGER_ADDRESSES[0]]: { + code: expect.any(String), + }, + }); + }); }); }); 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 ea9db48a3dc..0ebd23a8489 100644 --- a/packages/transaction-controller/src/utils/gas-fee-tokens.test.ts +++ b/packages/transaction-controller/src/utils/gas-fee-tokens.test.ts @@ -374,6 +374,44 @@ 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, + }, + }); + + await expect(getGasFeeTokens(request)).resolves.toStrictEqual({ + gasFeeTokens: [], + isGasFeeSponsored: 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, + }, + }); + + await expect(getGasFeeTokens(request)).resolves.toStrictEqual({ + gasFeeTokens: [], + isGasFeeSponsored: false, + }); + }); + it('forwards simulation config', async () => { const getSimulationConfigMock: GetSimulationConfig = jest.fn(); diff --git a/packages/transaction-controller/src/utils/prepare.test.ts b/packages/transaction-controller/src/utils/prepare.test.ts index a109bc13897..70e3eb5abe2 100644 --- a/packages/transaction-controller/src/utils/prepare.test.ts +++ b/packages/transaction-controller/src/utils/prepare.test.ts @@ -134,6 +134,7 @@ describe('Prepare Utils', () => { '0x0200567890123456789012345678901234567890123456789012345678901234', ); }); + }); }); diff --git a/packages/transaction-controller/src/utils/prepare.ts b/packages/transaction-controller/src/utils/prepare.ts index 1586f750984..a90969ec769 100644 --- a/packages/transaction-controller/src/utils/prepare.ts +++ b/packages/transaction-controller/src/utils/prepare.ts @@ -104,13 +104,7 @@ function normalizeAuthorizationList( * @returns The processed hexadecimal string. */ function removeLeadingZeroes(value: Hex | undefined): Hex | undefined { - if (!value) { - return value; - } - - if (value === '0x0') { - return '0x'; - } - - return (value.replace?.(/^0x(00)+/u, '0x') as Hex) ?? value; + return value === '0x0' + ? '0x' + : (value?.replace?.(/^0x(00)+/u, '0x') as Hex | undefined) ?? value; } From 330511daee4a8cf95b244516d11046a972cfde9e Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 10 Sep 2026 11:37:46 +0100 Subject: [PATCH 3/7] fix(transaction-controller): satisfy lint and changelog --- .../src/TransactionController.test.ts | 74 +++++++++++-------- .../src/TransactionController.ts | 14 ++-- .../src/utils/prepare.test.ts | 1 - .../src/utils/prepare.ts | 2 +- 4 files changed, 54 insertions(+), 37 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index bc79c99d474..9dc59e84c5b 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -975,12 +975,10 @@ describe('TransactionController', () => { }); const batchUpdateHandler = gasFeePollerMock.hub.on.mock.calls.find( ([event]) => event === 'transaction-batch-updated', - )?.[1] as ( - request: { - transactionBatchId: Hex; - gasFeeEstimates?: GasFeeEstimates; - }, - ) => void; + )?.[1] as (request: { + transactionBatchId: Hex; + gasFeeEstimates?: GasFeeEstimates; + }) => void; batchUpdateHandler({ transactionBatchId: batchId, @@ -2448,25 +2446,34 @@ describe('TransactionController', () => { it('calls isSponsored hook before reserving a nonce', async () => { const callOrder: string[] = []; - const isSponsoredHook = jest.fn().mockImplementation(async () => { - callOrder.push('isSponsored'); - expect(getNonceLockSpy).not.toHaveBeenCalled(); - return false; - }); + const isSponsoredHook = jest + .fn() + .mockImplementation(async (): Promise => { + callOrder.push('isSponsored'); + expect(getNonceLockSpy).not.toHaveBeenCalled(); + return false; + }); - const shouldSignHook = jest.fn().mockImplementation(async () => { - callOrder.push('shouldSign'); - expect(getNonceLockSpy).not.toHaveBeenCalled(); - return true; - }); + const shouldSignHook = jest + .fn() + .mockImplementation(async (): Promise => { + callOrder.push('shouldSign'); + expect(getNonceLockSpy).not.toHaveBeenCalled(); + return true; + }); - getNonceLockSpy.mockImplementation(async () => { - callOrder.push('getNonceLock'); - return { - nextNonce: NONCE_MOCK, - releaseLock: () => Promise.resolve(), - }; - }); + getNonceLockSpy.mockImplementation( + async (): Promise<{ + nextNonce: Hex; + releaseLock: () => Promise; + }> => { + callOrder.push('getNonceLock'); + return { + nextNonce: NONCE_MOCK, + releaseLock: () => Promise.resolve(), + }; + }, + ); const { controller } = setupController({ messengerOptions: { @@ -2496,7 +2503,11 @@ describe('TransactionController', () => { expect(isSponsoredHook).toHaveBeenCalledTimes(1); expect(shouldSignHook).toHaveBeenCalledTimes(1); - expect(callOrder).toStrictEqual(['isSponsored', 'shouldSign', 'getNonceLock']); + expect(callOrder).toStrictEqual([ + 'isSponsored', + 'shouldSign', + 'getNonceLock', + ]); }); it('skips nonce reservation when shouldSign resolves false', async () => { @@ -3962,13 +3973,16 @@ describe('TransactionController', () => { }); rootMessenger.unregisterActionHandler('AccountsController:getState'); - rootMessenger.registerActionHandler('AccountsController:getState', () => ({ - internalAccounts: { - accounts: { - [INTERNAL_ACCOUNT_MOCK.id]: INTERNAL_ACCOUNT_MOCK, + rootMessenger.registerActionHandler( + 'AccountsController:getState', + () => ({ + internalAccounts: { + accounts: { + [INTERNAL_ACCOUNT_MOCK.id]: INTERNAL_ACCOUNT_MOCK, + }, }, - }, - })); + }), + ); const { result } = await controller.addTransaction( { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK }, diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 098d22dc760..4e5420242d3 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -851,17 +851,20 @@ export class TransactionController extends BaseController< ((): Promise => Promise.resolve(true)); this.#isSponsored = hooks?.isSponsored ?? - (async ({ transactionMeta }: { transactionMeta: TransactionMeta }) => - Boolean(transactionMeta.isGasFeeSponsored)); + (async ({ + transactionMeta, + }: { + transactionMeta: TransactionMeta; + }): Promise => Boolean(transactionMeta.isGasFeeSponsored)); this.#shouldSign = hooks?.shouldSign ?? (async ({ transactionMeta, - isSponsored, + isSponsored: _isSponsored, }: { transactionMeta: TransactionMeta; isSponsored: boolean; - }) => !Boolean(transactionMeta.isExternalSign)); + }): Promise => !transactionMeta.isExternalSign); this.#beforePublish = hooks?.beforePublish ?? ((): Promise => Promise.resolve(true)); this.#beforeSign = @@ -3141,7 +3144,6 @@ export class TransactionController extends BaseController< clearApprovingTransactionId = (): boolean => this.#approvingTransactionIds.delete(transactionId); - // eslint-disable-next-line require-atomic-updates transactionMeta = this.#updateTransactionInternal( { transactionId, @@ -3162,6 +3164,7 @@ export class TransactionController extends BaseController< this.#onTransactionStatusChange(transactionMeta); + // eslint-disable-next-line require-atomic-updates transactionMeta = await this.#applyBeforeSignHook(transactionMeta); const { networkClientId } = transactionMeta; @@ -3176,6 +3179,7 @@ export class TransactionController extends BaseController< this.#updateTransactionInternal({ transactionId: txId }, fn), }); + // eslint-disable-next-line require-atomic-updates transactionMeta = this.#getTransactionOrThrow(transactionId); const isSponsored = await this.#isSponsored({ transactionMeta }); diff --git a/packages/transaction-controller/src/utils/prepare.test.ts b/packages/transaction-controller/src/utils/prepare.test.ts index 70e3eb5abe2..a109bc13897 100644 --- a/packages/transaction-controller/src/utils/prepare.test.ts +++ b/packages/transaction-controller/src/utils/prepare.test.ts @@ -134,7 +134,6 @@ describe('Prepare Utils', () => { '0x0200567890123456789012345678901234567890123456789012345678901234', ); }); - }); }); diff --git a/packages/transaction-controller/src/utils/prepare.ts b/packages/transaction-controller/src/utils/prepare.ts index a90969ec769..e437695d730 100644 --- a/packages/transaction-controller/src/utils/prepare.ts +++ b/packages/transaction-controller/src/utils/prepare.ts @@ -106,5 +106,5 @@ function normalizeAuthorizationList( function removeLeadingZeroes(value: Hex | undefined): Hex | undefined { return value === '0x0' ? '0x' - : (value?.replace?.(/^0x(00)+/u, '0x') as Hex | undefined) ?? value; + : ((value?.replace?.(/^0x(00)+/u, '0x') as Hex | undefined) ?? value); } From c670baa5039fa6e25b7aedff43037cf63eae1225 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 10 Sep 2026 12:15:28 +0100 Subject: [PATCH 4/7] fix(transaction-controller): align lint and changelog --- packages/transaction-controller/CHANGELOG.md | 27 +++++++++++++++++-- .../src/utils/gas-fee-tokens.test.ts | 4 +-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index a560f87f90d..14bcb589c10 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Uncategorized + +- fix(transaction-controller): satisfy lint and changelog +- test(transaction-controller): cover approval edge cases +- docs(transaction-controller): concise approval hook changelog +- fix(transaction-controller): restore gas fee preflight on approval +- chore: Migrate SIWE dependency to @signinwithethereum/siwe v4 ([#10049](https://github.com/MetaMask/core/pull/10049)) +- Release 1245.0.0 ([#10146](https://github.com/MetaMask/core/pull/10146)) +- feat(perps): support explicit HyperLiquid margin mode ([#10136](https://github.com/MetaMask/core/pull/10136)) +- Release 1244.0.0 ([#10145](https://github.com/MetaMask/core/pull/10145)) +- fix(kyc-controller): treat native SumSub completion and applicant abandonment separately ([#10133](https://github.com/MetaMask/core/pull/10133)) +- fix(notification-services-controller): prevent unsibscribing if keyring is locked ([#10143](https://github.com/MetaMask/core/pull/10143)) +- Release 1243.0.0 ([#10141](https://github.com/MetaMask/core/pull/10141)) +- chore: disable persistence for legacy assets controllers ([#9842](https://github.com/MetaMask/core/pull/9842)) +- fix(perps-controller): keep the terminal TWAP record when a completing fill ties lastUpdated ([#10122](https://github.com/MetaMask/core/pull/10122)) +- fix(perps): stabilize Lighter account loading ([#10119](https://github.com/MetaMask/core/pull/10119)) +- fix: filter Price API v3 requests to supported networks ([#10132](https://github.com/MetaMask/core/pull/10132)) +- fix(assets-controller): make #start() re-entrancy safe ([#10131](https://github.com/MetaMask/core/pull/10131)) +- feat(profile-sync-controller): add social pairing support ([#10128](https://github.com/MetaMask/core/pull/10128)) +- Release 1240.0.0 ([#10137](https://github.com/MetaMask/core/pull/10137)) +- feat(account-tree-controller)!: add `strip{Metadata,Secrets}` helpers on state snapshot ([#10112](https://github.com/MetaMask/core/pull/10112)) +- feat: gate Relay source sponsorship on payer capability ([#10126](https://github.com/MetaMask/core/pull/10126)) +- fix: keep persisted feature flags for empty segmentation id ([#10123](https://github.com/MetaMask/core/pull/10123)) + ### Changed - Bump `uuid` from `^8.3.2` to `^9.0.1` ([#10117](https://github.com/MetaMask/core/pull/10117)) @@ -2753,8 +2777,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 All changes listed after this point were applied to this package following the monorepo conversion. -[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@70.0.0...HEAD -[70.0.0]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@69.8.1...@metamask/transaction-controller@70.0.0 +[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@69.8.1...HEAD [69.8.1]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@69.8.0...@metamask/transaction-controller@69.8.1 [69.8.0]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@69.7.0...@metamask/transaction-controller@69.8.0 [69.7.0]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@69.6.1...@metamask/transaction-controller@69.7.0 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 0ebd23a8489..99ee4fd3fe5 100644 --- a/packages/transaction-controller/src/utils/gas-fee-tokens.test.ts +++ b/packages/transaction-controller/src/utils/gas-fee-tokens.test.ts @@ -387,7 +387,7 @@ describe('Gas Fee Tokens Utils', () => { }, }); - await expect(getGasFeeTokens(request)).resolves.toStrictEqual({ + expect(await getGasFeeTokens(request)).toStrictEqual({ gasFeeTokens: [], isGasFeeSponsored: false, }); @@ -406,7 +406,7 @@ describe('Gas Fee Tokens Utils', () => { }, }); - await expect(getGasFeeTokens(request)).resolves.toStrictEqual({ + expect(await getGasFeeTokens(request)).toStrictEqual({ gasFeeTokens: [], isGasFeeSponsored: false, }); From 7afca331b8818b3394f70317ba6186ddbde71f0b Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 10 Sep 2026 18:18:10 +0100 Subject: [PATCH 5/7] fix(transaction-controller): satisfy lint and changelog --- packages/transaction-controller/CHANGELOG.md | 27 ++----------------- .../src/TransactionController.ts | 19 ++++++------- 2 files changed, 12 insertions(+), 34 deletions(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 14bcb589c10..a560f87f90d 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,30 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Uncategorized - -- fix(transaction-controller): satisfy lint and changelog -- test(transaction-controller): cover approval edge cases -- docs(transaction-controller): concise approval hook changelog -- fix(transaction-controller): restore gas fee preflight on approval -- chore: Migrate SIWE dependency to @signinwithethereum/siwe v4 ([#10049](https://github.com/MetaMask/core/pull/10049)) -- Release 1245.0.0 ([#10146](https://github.com/MetaMask/core/pull/10146)) -- feat(perps): support explicit HyperLiquid margin mode ([#10136](https://github.com/MetaMask/core/pull/10136)) -- Release 1244.0.0 ([#10145](https://github.com/MetaMask/core/pull/10145)) -- fix(kyc-controller): treat native SumSub completion and applicant abandonment separately ([#10133](https://github.com/MetaMask/core/pull/10133)) -- fix(notification-services-controller): prevent unsibscribing if keyring is locked ([#10143](https://github.com/MetaMask/core/pull/10143)) -- Release 1243.0.0 ([#10141](https://github.com/MetaMask/core/pull/10141)) -- chore: disable persistence for legacy assets controllers ([#9842](https://github.com/MetaMask/core/pull/9842)) -- fix(perps-controller): keep the terminal TWAP record when a completing fill ties lastUpdated ([#10122](https://github.com/MetaMask/core/pull/10122)) -- fix(perps): stabilize Lighter account loading ([#10119](https://github.com/MetaMask/core/pull/10119)) -- fix: filter Price API v3 requests to supported networks ([#10132](https://github.com/MetaMask/core/pull/10132)) -- fix(assets-controller): make #start() re-entrancy safe ([#10131](https://github.com/MetaMask/core/pull/10131)) -- feat(profile-sync-controller): add social pairing support ([#10128](https://github.com/MetaMask/core/pull/10128)) -- Release 1240.0.0 ([#10137](https://github.com/MetaMask/core/pull/10137)) -- feat(account-tree-controller)!: add `strip{Metadata,Secrets}` helpers on state snapshot ([#10112](https://github.com/MetaMask/core/pull/10112)) -- feat: gate Relay source sponsorship on payer capability ([#10126](https://github.com/MetaMask/core/pull/10126)) -- fix: keep persisted feature flags for empty segmentation id ([#10123](https://github.com/MetaMask/core/pull/10123)) - ### Changed - Bump `uuid` from `^8.3.2` to `^9.0.1` ([#10117](https://github.com/MetaMask/core/pull/10117)) @@ -2777,7 +2753,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 All changes listed after this point were applied to this package following the monorepo conversion. -[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@69.8.1...HEAD +[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@70.0.0...HEAD +[70.0.0]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@69.8.1...@metamask/transaction-controller@70.0.0 [69.8.1]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@69.8.0...@metamask/transaction-controller@69.8.1 [69.8.0]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@69.7.0...@metamask/transaction-controller@69.8.0 [69.7.0]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@69.6.1...@metamask/transaction-controller@69.7.0 diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 4e5420242d3..3eccc700fce 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -1272,7 +1272,8 @@ export class TransactionController extends BaseController< } else { const newTransactionMeta = cloneDeep(addedTransactionMeta); - this.#updateGasProperties(newTransactionMeta) + // eslint-disable-next-line no-void + void this.#updateGasProperties(newTransactionMeta) .then(() => { this.#updateTransactionInternal( { @@ -1308,7 +1309,8 @@ export class TransactionController extends BaseController< this.#addMetadata(addedTransactionMeta); - delegationAddressPromise + // eslint-disable-next-line no-void + void delegationAddressPromise .then((delegationAddress) => { this.#updateTransactionInternal( { @@ -2418,21 +2420,20 @@ export class TransactionController extends BaseController< pickBy(transactionsToFilter, (transaction) => { // iterate over the predicateMethods keys to check if the transaction // matches the searchCriteria + const txParams = transaction.txParams as Record; + const txMeta = transaction as Record; + for (const [key, predicate] of Object.entries(predicateMethods)) { // We return false early as soon as we know that one of the specified // search criteria do not match the transaction. This prevents // needlessly checking all criteria when we already know the criteria // are not fully satisfied. We check both txParams and the base // object as predicate keys can be either. - if (key in transaction.txParams) { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - if (predicate((transaction.txParams as any)[key]) === false) { + if (key in txParams) { + if (predicate(txParams[key]) === false) { return false; } - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } else if (predicate((transaction as any)[key]) === false) { + } else if (predicate(txMeta[key]) === false) { return false; } } From e9262f4fcd763d9ee1b6c28eea2b4f8868053e73 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 10 Sep 2026 18:32:03 +0100 Subject: [PATCH 6/7] fix(transaction-controller): publish approval status after nonce --- packages/transaction-controller/CHANGELOG.md | 5 --- .../src/TransactionController.test.ts | 37 +++++++++++++++++++ .../src/TransactionController.ts | 6 +-- 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index a560f87f90d..94d8adb7e4e 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -34,11 +34,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [69.8.1] -### Changed - -- Bump `@metamask/core-backend` from `^9.0.0` to `^9.1.1` ([#10138](https://github.com/MetaMask/core/pull/10138), [#10139](https://github.com/MetaMask/core/pull/10139)) -- Bump `@metamask/remote-feature-flag-controller` from `^6.1.0` to `^6.1.1` ([#10129](https://github.com/MetaMask/core/pull/10129)) - ### Fixed - Harden gas fee token preflight by not treating pending gas estimates as zero-cost native gas, and by resetting `isExternalSign` when preflight validation fails ([#10071](https://github.com/MetaMask/core/pull/10071)) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 9dc59e84c5b..add20755604 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -9239,6 +9239,43 @@ 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 3eccc700fce..aa20e434404 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -3163,8 +3163,6 @@ export class TransactionController extends BaseController< }, ); - this.#onTransactionStatusChange(transactionMeta); - // eslint-disable-next-line require-atomic-updates transactionMeta = await this.#applyBeforeSignHook(transactionMeta); @@ -3228,8 +3226,6 @@ export class TransactionController extends BaseController< }, ); - this.#onTransactionStatusChange(transactionMeta); - rawTx = await this.#trace( { name: 'Sign', parentContext: traceContext }, () => this.#signTransaction(transactionMeta, true, true), @@ -3239,6 +3235,8 @@ export class TransactionController extends BaseController< transactionMeta = this.#getTransactionOrThrow(transactionId); } + this.#onTransactionStatusChange(transactionMeta); + if (!(await this.#beforePublish(transactionMeta))) { log('Skipping publishing transaction based on hook'); this.messenger.publish( From 58ac50582e59a9e6f94ded96396485bcdec73e23 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Fri, 11 Sep 2026 11:05:46 +0100 Subject: [PATCH 7/7] style: format transaction controller test --- .../src/TransactionController.test.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index add20755604..3ef38d9de73 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -9241,11 +9241,8 @@ describe('TransactionController', () => { }); it('publishes transactionApproved with a nonce after signing approval', async () => { - const { - controller, - messenger, - mockTransactionApprovalRequest, - } = setupController(); + const { controller, messenger, mockTransactionApprovalRequest } = + setupController(); const approvedEventListener = jest.fn();