From 0db5d602eb4577dcceac4796de2c14094e8b4428 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 16 Jul 2026 17:07:11 +0100 Subject: [PATCH 01/11] feat: optimize transaction pay amount quote pipeline --- packages/transaction-controller/CHANGELOG.md | 3 + ...ansactionController-method-action-types.ts | 13 + .../src/TransactionController.test.ts | 241 +++++++++++++ .../src/TransactionController.ts | 204 +++++++++-- packages/transaction-controller/src/index.ts | 5 + packages/transaction-controller/src/types.ts | 50 +++ .../transaction-pay-controller/CHANGELOG.md | 3 + ...actionPayController-method-action-types.ts | 15 + .../src/TransactionPayController.test.ts | 332 +++++++++++++++++- .../src/TransactionPayController.ts | 176 +++++++++- .../transaction-pay-controller/src/index.ts | 5 + .../src/strategy/relay/relay-quotes.test.ts | 36 ++ .../src/strategy/relay/relay-quotes.ts | 21 +- .../src/tests/messenger-mock.ts | 11 + .../transaction-pay-controller/src/types.ts | 60 ++++ .../src/utils/quotes.test.ts | 40 ++- .../src/utils/quotes.ts | 55 ++- 17 files changed, 1227 insertions(+), 43 deletions(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 01fce8cb00e..d7ff2f13171 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -29,6 +29,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Query layer 1 gas fee oracles via direct `eth_call` RPC requests instead of an ethers `Contract` backed by `Web3Provider` ([#9505](https://github.com/MetaMask/core/pull/9505)) - `Web3Provider` schedules its JSON-RPC dispatch with `setTimeout`, which never fires on React Native iOS when the timer pump is starved, blocking `addTransaction` indefinitely and preventing dapp confirmations from appearing ([MetaMask/metamask-mobile#32863](https://github.com/MetaMask/metamask-mobile/issues/32863)) +### Added + +- Add `beginAtomicBatchUpdate` for coherent multi-call updates with monotonic revisions and revision-bound gas preparation ## [69.0.0] diff --git a/packages/transaction-controller/src/TransactionController-method-action-types.ts b/packages/transaction-controller/src/TransactionController-method-action-types.ts index 194d8a98d4e..8cd278cc90f 100644 --- a/packages/transaction-controller/src/TransactionController-method-action-types.ts +++ b/packages/transaction-controller/src/TransactionController-method-action-types.ts @@ -358,6 +358,18 @@ export type TransactionControllerAbortTransactionSigningAction = { handler: TransactionController['abortTransactionSigning']; }; +/** + * Atomically updates all amount-dependent data in an atomic batch and starts + * revision-bound local preparation. + * + * @param request - Complete atomic batch update. + * @returns The synchronously published revision and its preparation handle. + */ +export type TransactionControllerBeginAtomicBatchUpdateAction = { + type: `TransactionController:beginAtomicBatchUpdate`; + handler: TransactionController['beginAtomicBatchUpdate']; +}; + /** * Update the transaction data of a single nested transaction within an atomic batch transaction. * @@ -457,6 +469,7 @@ export type TransactionControllerMethodActions = | TransactionControllerGetLayer1GasFeeAction | TransactionControllerClearUnapprovedTransactionsAction | TransactionControllerAbortTransactionSigningAction + | TransactionControllerBeginAtomicBatchUpdateAction | TransactionControllerUpdateAtomicBatchDataAction | TransactionControllerUpdateSelectedGasFeeTokenAction | TransactionControllerUpdateRequiredTransactionIdsAction diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 4a0bb2b2107..85d38e5b13b 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -7618,6 +7618,247 @@ describe('TransactionController', () => { }); }); + describe('beginAtomicBatchUpdate', () => { + const requiredAssets = [ + { + address: ACCOUNT_2_MOCK, + amount: '0x64' as Hex, + standard: 'erc20', + }, + ]; + + function setupAtomicBatchController( + transactionOverrides: Partial = {}, + ): ReturnType { + return setupController({ + options: { + state: { + transactions: [ + { + ...TRANSACTION_META_MOCK, + status: TransactionStatus.unapproved, + nestedTransactions: [ + { to: ACCOUNT_2_MOCK, data: '0x1234' }, + { to: ACCOUNT_2_MOCK, data: '0x4567' }, + ], + ...transactionOverrides, + }, + ], + }, + }, + updateToInitialState: true, + }); + } + + it('publishes all nested updates and required assets synchronously in one revision', async () => { + const { controller } = setupAtomicBatchController(); + + const result = controller.beginAtomicBatchUpdate({ + transactionId: TRANSACTION_META_MOCK.id, + requiredAssets, + nestedTransactionUpdates: [ + { transactionIndex: 0, transactionData: '0xAAAA' }, + { transactionIndex: 1, transactionData: '0xBBBB' }, + ], + }); + + expect(result).toStrictEqual({ + revision: 1, + transaction: expect.objectContaining({ + requiredAssets, + transactionRevision: 1, + }), + preparation: expect.any(Promise), + }); + expect( + controller.state.transactions[0].nestedTransactions?.map( + ({ data }) => data, + ), + ).toStrictEqual(['0xAAAA', '0xBBBB']); + expect(controller.state.transactions[0].txParams.data).toContain('aaaa'); + expect(controller.state.transactions[0].txParams.data).toContain('bbbb'); + + expect(await result.preparation).toMatchObject({ + revision: 1, + status: 'prepared', + }); + expect(updateGasMock).toHaveBeenCalledTimes(1); + }); + + it.each([ + { + expectedError: 'at least one nested transaction update', + name: 'an empty patch', + updates: [], + }, + { + expectedError: 'Duplicate nested transaction index - 0', + name: 'duplicate indexes', + updates: [ + { transactionIndex: 0, transactionData: '0xAAAA' as Hex }, + { transactionIndex: 0, transactionData: '0xBBBB' as Hex }, + ], + }, + { + expectedError: 'Nested transaction not found with index - 2', + name: 'an invalid index', + updates: [{ transactionIndex: 2, transactionData: '0xAAAA' as Hex }], + }, + ])( + 'rejects $name without publishing state', + ({ expectedError, updates }) => { + const { controller } = setupAtomicBatchController(); + const stateBeforeUpdate = controller.state; + + expect(() => + controller.beginAtomicBatchUpdate({ + transactionId: TRANSACTION_META_MOCK.id, + requiredAssets, + nestedTransactionUpdates: updates, + }), + ).toThrow(expectedError); + expect(controller.state).toBe(stateBeforeUpdate); + expect(updateGasMock).not.toHaveBeenCalled(); + }, + ); + + it('clears stale revision-bound preparation metadata while preserving non-gas reverts', async () => { + const gasPreparation = createDeferredPromise(); + const simulationRevert = { message: 'Simulation reverted' }; + const receiptRevert = { message: 'Receipt reverted' }; + updateGasMock.mockImplementationOnce(async () => { + await gasPreparation.promise; + }); + const { controller } = setupAtomicBatchController({ + gasLimitNoBuffer: '0x111', + gasUsed: '0x222', + revert: { + gas: { message: 'Gas reverted' }, + receipt: receiptRevert, + simulation: simulationRevert, + }, + securityAlertResponse: { + reason: 'Previous revision warning', + result_type: 'Warning', + }, + simulationData: SIMULATION_DATA_RESULT_MOCK, + simulationFails: { + debug: {}, + reason: 'Previous gas estimate failed', + }, + txParams: { + ...TRANSACTION_META_MOCK.txParams, + gas: '0x333', + }, + }); + + const result = controller.beginAtomicBatchUpdate({ + transactionId: TRANSACTION_META_MOCK.id, + requiredAssets, + nestedTransactionUpdates: [ + { transactionIndex: 0, transactionData: '0xAAAA' }, + ], + }); + const transaction = controller.state.transactions[0]; + + expect(transaction.txParams.gas).toBeUndefined(); + expect(transaction.gasLimitNoBuffer).toBeUndefined(); + expect(transaction.gasUsed).toBeUndefined(); + expect(transaction.simulationData).toBeUndefined(); + expect(transaction.simulationFails).toBeUndefined(); + expect(transaction.securityAlertResponse).toBeUndefined(); + expect(transaction.revert).toStrictEqual({ + receipt: receiptRevert, + simulation: simulationRevert, + }); + + gasPreparation.resolve(); + await result.preparation; + }); + + it('ignores simulation results started before the current revision', async () => { + const staleSimulation = createDeferredPromise<{ + simulationData: SimulationData; + }>(); + const { controller } = setupAtomicBatchController(); + getBalanceChangesMock.mockReturnValueOnce(staleSimulation.promise); + shouldResimulateMock.mockReturnValueOnce({ + blockTime: 123, + resimulate: true, + }); + + await controller.updateEditableParams(TRANSACTION_META_MOCK.id, {}); + expect(getBalanceChangesMock).toHaveBeenCalledTimes(1); + + const update = controller.beginAtomicBatchUpdate({ + transactionId: TRANSACTION_META_MOCK.id, + requiredAssets, + nestedTransactionUpdates: [ + { transactionIndex: 0, transactionData: '0xAAAA' }, + ], + }); + await update.preparation; + + staleSimulation.resolve({ + simulationData: { + ...SIMULATION_DATA_RESULT_MOCK, + nativeBalanceChange: undefined, + }, + }); + await flushPromises(); + + expect(controller.state.transactions[0].simulationData).toStrictEqual( + SIMULATION_DATA_RESULT_MOCK, + ); + expect(controller.state.transactions[0].transactionRevision).toBe(1); + }); + + it('increments revisions and prevents stale gas from overwriting a newer revision', async () => { + const firstGas = createDeferredPromise(); + const { controller } = setupAtomicBatchController(); + + updateGasMock + .mockImplementationOnce(async ({ txMeta }) => { + await firstGas.promise; + txMeta.txParams.gas = '0x111'; + }) + .mockImplementationOnce(async ({ txMeta }) => { + txMeta.txParams.gas = '0x222'; + }); + + const first = controller.beginAtomicBatchUpdate({ + transactionId: TRANSACTION_META_MOCK.id, + requiredAssets, + nestedTransactionUpdates: [ + { transactionIndex: 0, transactionData: '0xAAAA' }, + ], + }); + const second = controller.beginAtomicBatchUpdate({ + transactionId: TRANSACTION_META_MOCK.id, + requiredAssets, + nestedTransactionUpdates: [ + { transactionIndex: 0, transactionData: '0xBBBB' }, + ], + }); + + expect(await second.preparation).toMatchObject({ + revision: 2, + status: 'prepared', + }); + firstGas.resolve(); + expect(await first.preparation).toMatchObject({ + revision: 1, + status: 'superseded', + }); + + expect(controller.state.transactions[0].transactionRevision).toBe(2); + expect(controller.state.transactions[0].txParams.gas).toBe('0x222'); + expect( + controller.state.transactions[0].nestedTransactions?.[0].data, + ).toBe('0xBBBB'); + }); + }); + describe('updateAtomicBatchData', () => { /** * Template for updateAtomicBatchData test. diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index ea765290515..5acf1980697 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -129,6 +129,9 @@ import type { AddTransactionOptions, PublishHookResult, GetGasFeeTokensRequest, + BeginAtomicBatchUpdateRequest, + BeginAtomicBatchUpdateResult, + AtomicBatchPreparationResult, } from './types.js'; import { GasFeeEstimateLevel, @@ -675,6 +678,7 @@ const MESSENGER_EXPOSED_METHODS = [ 'addTransaction', 'addTransactionBatch', 'approveTransactionsWithSameNonce', + 'beginAtomicBatchUpdate', 'clearUnapprovedTransactions', 'confirmExternalTransaction', 'emulateNewTransaction', @@ -2556,6 +2560,119 @@ export class TransactionController extends BaseController< this.#signAbortCallbacks.delete(transactionId); } + /** + * Atomically updates all amount-dependent data in an atomic batch and starts + * revision-bound local preparation. + * + * @param request - Complete atomic batch update. + * @returns The synchronously published revision and its preparation handle. + */ + beginAtomicBatchUpdate( + request: BeginAtomicBatchUpdateRequest, + ): BeginAtomicBatchUpdateResult { + const { transactionId, requiredAssets, nestedTransactionUpdates } = request; + const currentTransaction = this.#getTransaction(transactionId); + let revision = 0; + + if (!currentTransaction) { + throw new Error( + `Cannot update transaction as ID not found - ${transactionId}`, + ); + } + + if (nestedTransactionUpdates.length === 0) { + throw new Error( + 'Atomic batch update requires at least one nested transaction update', + ); + } + + const updateIndexes = new Set(); + + for (const { transactionIndex } of nestedTransactionUpdates) { + if (updateIndexes.has(transactionIndex)) { + throw new Error( + `Duplicate nested transaction index - ${transactionIndex}`, + ); + } + + if (!currentTransaction.nestedTransactions?.[transactionIndex]) { + throw new Error( + `Nested transaction not found with index - ${transactionIndex}`, + ); + } + + updateIndexes.add(transactionIndex); + } + + log('Beginning atomic batch update', request); + + const updatedTransactionMeta = this.#updateTransactionInternal( + { transactionId, skipResimulateCheck: true }, + (transactionMeta) => { + const { nestedTransactions, txParams } = transactionMeta; + const from = txParams.from as Hex; + + for (const { + transactionIndex, + transactionData, + } of nestedTransactionUpdates) { + const nestedTransaction = nestedTransactions?.[transactionIndex]; + + if (!nestedTransaction) { + throw new Error( + `Nested transaction not found with index - ${transactionIndex}`, + ); + } + + nestedTransaction.data = transactionData; + } + + const batchTransaction = generateEIP7702BatchTransaction( + from, + nestedTransactions ?? [], + ); + + revision = (transactionMeta.transactionRevision ?? 0) + 1; + transactionMeta.requiredAssets = requiredAssets; + transactionMeta.transactionRevision = revision; + transactionMeta.txParams.data = batchTransaction.data; + transactionMeta.txParams.gas = undefined; + transactionMeta.gasLimitNoBuffer = undefined; + transactionMeta.gasUsed = undefined; + transactionMeta.securityAlertResponse = undefined; + transactionMeta.simulationData = undefined; + transactionMeta.simulationFails = undefined; + + if (transactionMeta.revert) { + delete transactionMeta.revert.gas; + + if ( + !transactionMeta.revert.simulation && + !transactionMeta.revert.receipt + ) { + transactionMeta.revert = undefined; + } + } + }, + ); + + const transaction = cloneDeep(updatedTransactionMeta); + const draftTransaction = cloneDeep({ + ...transaction, + txParams: { + ...transaction.txParams, + // Clear existing gas to force estimation. + gas: undefined, + }, + }); + + return { + revision, + transaction, + preparation: this.#prepareAtomicBatchUpdate(draftTransaction, revision), + }; + } + /** * Update the transaction data of a single nested transaction within an atomic batch transaction. * @@ -2580,46 +2697,50 @@ export class TransactionController extends BaseController< transactionData, }); - const updatedTransactionMeta = this.#updateTransactionInternal( - { - transactionId, - }, - (transactionMeta) => { - const { nestedTransactions, txParams } = transactionMeta; - const from = txParams.from as Hex; - const nestedTransaction = nestedTransactions?.[transactionIndex]; - - if (!nestedTransaction) { - throw new Error( - `Nested transaction not found with index - ${transactionIndex}`, - ); - } + const currentTransaction = this.#getTransaction(transactionId); - nestedTransaction.data = transactionData; + if (!currentTransaction) { + throw new Error( + `Cannot update transaction as ID not found - ${transactionId}`, + ); + } - const batchTransaction = generateEIP7702BatchTransaction( - from, - nestedTransactions, - ); + const { preparation, transaction } = this.beginAtomicBatchUpdate({ + transactionId, + requiredAssets: currentTransaction.requiredAssets ?? [], + nestedTransactionUpdates: [{ transactionIndex, transactionData }], + }); - transactionMeta.txParams.data = batchTransaction.data; - }, - ); + await preparation; - const draftTransaction = cloneDeep({ - ...updatedTransactionMeta, - txParams: { - ...updatedTransactionMeta.txParams, - // Clear existing gas to force estimation - gas: undefined, - }, - }); + return transaction.txParams.data as Hex; + } - await this.#updateGasEstimate(draftTransaction); + async #prepareAtomicBatchUpdate( + draftTransaction: TransactionMeta, + revision: number, + ): Promise { + await Promise.all([ + this.#updateGasEstimate(draftTransaction), + this.#isSimulationEnabled() + ? this.#updateSimulationData(draftTransaction) + : Promise.resolve(), + ]); + + const currentTransaction = this.#getTransaction(draftTransaction.id); + + if (currentTransaction?.transactionRevision !== revision) { + return { + revision, + status: 'superseded', + transaction: cloneDeep(currentTransaction ?? draftTransaction), + }; + } - this.#updateTransactionInternal( + const preparedTransaction = this.#updateTransactionInternal( { - transactionId, + transactionId: draftTransaction.id, + skipResimulateCheck: true, }, (transactionMeta) => { transactionMeta.txParams.gas = draftTransaction.txParams.gas; @@ -2636,7 +2757,11 @@ export class TransactionController extends BaseController< }, ); - return updatedTransactionMeta.txParams.data as Hex; + return { + revision, + status: 'prepared', + transaction: cloneDeep(preparedTransaction), + }; } /** @@ -4125,6 +4250,17 @@ export class TransactionController extends BaseController< return; } + if ( + latestTransactionMeta.transactionRevision !== + transactionMeta.transactionRevision + ) { + log('Ignoring stale simulation data', { + transactionId, + revision: transactionMeta.transactionRevision, + }); + return; + } + const updatedTransactionMeta = this.#updateTransactionInternal( { transactionId, diff --git a/packages/transaction-controller/src/index.ts b/packages/transaction-controller/src/index.ts index cb5f7c4fd01..8034d9f11c7 100644 --- a/packages/transaction-controller/src/index.ts +++ b/packages/transaction-controller/src/index.ts @@ -49,6 +49,7 @@ export type { TransactionControllerGetLayer1GasFeeAction, TransactionControllerClearUnapprovedTransactionsAction, TransactionControllerAbortTransactionSigningAction, + TransactionControllerBeginAtomicBatchUpdateAction, TransactionControllerUpdateAtomicBatchDataAction, TransactionControllerWipeTransactionsAction, TransactionControllerUpdateSecurityAlertResponseAction, @@ -66,7 +67,10 @@ export type { AddTransactionOptions, AfterAddHook, Authorization, + AtomicBatchPreparationResult, AuthorizationList, + BeginAtomicBatchUpdateRequest, + BeginAtomicBatchUpdateResult, BatchTransaction, BatchTransactionParams, BeforeSignHook, @@ -89,6 +93,7 @@ export type { Log, MetamaskPayMetadata, NestedTransactionMetadata, + NestedTransactionUpdate, PublishBatchHook, PublishBatchHookRequest, PublishBatchHookResult, diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index 68c01373204..4aabe98538c 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -265,6 +265,11 @@ export type TransactionMeta = { */ id: string; + /** + * Monotonic revision assigned whenever the atomic batch calldata is updated. + */ + transactionRevision?: number; + /** * Whether the transaction is signed externally. * No signing will be performed in the client and the `nonce` will be `undefined`. @@ -2331,6 +2336,51 @@ export type RequiredAsset = { standard: string; }; +/** A nested transaction calldata update in an atomic batch. */ +export type NestedTransactionUpdate = { + /** Index of the nested transaction to update. */ + transactionIndex: number; + + /** New calldata for the nested transaction. */ + transactionData: Hex; +}; + +/** Request to atomically update all amount-dependent batch data. */ +export type BeginAtomicBatchUpdateRequest = { + /** ID of the atomic batch transaction. */ + transactionId: string; + + /** Complete assets required by the updated transaction. */ + requiredAssets: RequiredAsset[]; + + /** Complete set of nested transaction calldata updates. */ + nestedTransactionUpdates: NestedTransactionUpdate[]; +}; + +/** Result of revision-bound local preparation. */ +export type AtomicBatchPreparationResult = { + /** Revision for which preparation ran. */ + revision: number; + + /** Whether the prepared metadata was committed or superseded. */ + status: 'prepared' | 'superseded'; + + /** Prepared transaction, or the current transaction if superseded. */ + transaction: TransactionMeta; +}; + +/** Synchronous result returned when an atomic update begins. */ +export type BeginAtomicBatchUpdateResult = { + /** Monotonic transaction revision assigned to the update. */ + revision: number; + + /** Coherent transaction snapshot published for this revision. */ + transaction: TransactionMeta; + + /** Revision-bound local gas preparation. */ + preparation: Promise; +}; + /** * Decoded revert from a single lifecycle source. */ diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index 6491a8e01a0..06f56f4660a 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -49,6 +49,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Consume `hasTransactionType` helper from `@metamask/transaction-controller` to derive relevant transaction type against the top-level `TransactionMeta` ([#9570](https://github.com/MetaMask/core/pull/9570)) - Bump `@metamask/transaction-controller` from `^69.0.0` to `^69.2.0` ([#9568](https://github.com/MetaMask/core/pull/9568), [#9589](https://github.com/MetaMask/core/pull/9589)) - Bump `@metamask/assets-controller` from `^11.0.0` to `^11.1.0` ([#9579](https://github.com/MetaMask/core/pull/9579)) +### Added + +- Add explicit `updateAmount` orchestration with complete-patch validation, in-flight intent deduplication, and revision-bound Relay quote publication ## [25.0.0] diff --git a/packages/transaction-pay-controller/src/TransactionPayController-method-action-types.ts b/packages/transaction-pay-controller/src/TransactionPayController-method-action-types.ts index 14a91436fa2..c658439b938 100644 --- a/packages/transaction-pay-controller/src/TransactionPayController-method-action-types.ts +++ b/packages/transaction-pay-controller/src/TransactionPayController-method-action-types.ts @@ -19,6 +19,20 @@ export type TransactionPayControllerSetTransactionConfigAction = { handler: TransactionPayController['setTransactionConfig']; }; +/** + * Prepares and atomically commits an exact transaction amount, then launches + * one quote generation joined to revision-bound local preparation. + * Identical in-flight intents share the same promise; different intents + * supersede and abort earlier work. + * + * @param request - Exact amount and transaction ID. + * @returns Whether the matching quote generation was published. + */ +export type TransactionPayControllerUpdateAmountAction = { + type: `TransactionPayController:updateAmount`; + handler: TransactionPayController['updateAmount']; +}; + /** * Updates the payment token for a transaction. * @@ -142,6 +156,7 @@ export type TransactionPayControllerPolymarketSubmitDepositWalletBatchAction = { */ export type TransactionPayControllerMethodActions = | TransactionPayControllerSetTransactionConfigAction + | TransactionPayControllerUpdateAmountAction | TransactionPayControllerUpdatePaymentTokenAction | TransactionPayControllerUpdateFiatPaymentAction | TransactionPayControllerGetDelegationTransactionAction diff --git a/packages/transaction-pay-controller/src/TransactionPayController.test.ts b/packages/transaction-pay-controller/src/TransactionPayController.test.ts index 47ebeb8d12f..012f0ad6769 100644 --- a/packages/transaction-pay-controller/src/TransactionPayController.test.ts +++ b/packages/transaction-pay-controller/src/TransactionPayController.test.ts @@ -1,7 +1,11 @@ /* eslint-disable no-new */ -import type { TransactionMeta } from '@metamask/transaction-controller'; -import type { Hex } from '@metamask/utils'; +import type { + BeginAtomicBatchUpdateResult, + TransactionMeta, +} from '@metamask/transaction-controller'; +import type { Hex, Json } from '@metamask/utils'; +import { createDeferredPromise } from '@metamask/utils'; import { updateFiatPayment } from './actions/update-fiat-payment.js'; import { updatePaymentToken } from './actions/update-payment-token.js'; @@ -10,9 +14,12 @@ import { TransactionPayController } from './index.js'; import { deriveFiatAssetForFiatPayment } from './strategy/fiat/utils.js'; import { getMessengerMock } from './tests/messenger-mock.js'; import type { + PrepareTransactionAmountResult, TransactionPayControllerMessenger, TransactionPayControllerOptions, + TransactionPayQuote, TransactionPaySourceAmount, + TransactionPayTotals, UpdateTransactionDataCallback, } from './types.js'; import { getStrategyOrder } from './utils/feature-flags.js'; @@ -51,6 +58,7 @@ describe('TransactionPayController', () => { const subscribeAssetChangesMock = jest.mocked(subscribeAssetChanges); const getStrategyOrderMock = jest.mocked(getStrategyOrder); let messenger: TransactionPayControllerMessenger; + let beginAtomicBatchUpdateMock: jest.Mock; let getKeyringControllerStateMock: jest.Mock; /** @@ -74,6 +82,7 @@ describe('TransactionPayController', () => { const mocks = getMessengerMock({ skipRegister: true }); messenger = mocks.messenger; + beginAtomicBatchUpdateMock = mocks.beginAtomicBatchUpdateMock; getKeyringControllerStateMock = mocks.getKeyringControllerStateMock; getKeyringControllerStateMock.mockReturnValue({ @@ -106,6 +115,325 @@ describe('TransactionPayController', () => { }); }); + describe('updateAmount', () => { + const transaction = { + id: TRANSACTION_ID_MOCK, + nestedTransactions: [ + { data: '0x1111' as Hex }, + { data: '0x2222' as Hex }, + ], + txParams: { from: '0x1234567890123456789012345678901234567891' }, + } as TransactionMeta; + const requiredAssets = [ + { + address: '0x1234567890123456789012345678901234567892' as Hex, + amount: '0x64' as Hex, + standard: 'erc20', + }, + ]; + const nestedTransactionUpdates = [ + { transactionIndex: 0, transactionData: '0xAAAA' as Hex }, + { transactionIndex: 1, transactionData: '0xBBBB' as Hex }, + ]; + + function mockAtomicUpdate(): BeginAtomicBatchUpdateResult { + const result = { + revision: 1, + transaction: { ...transaction, transactionRevision: 1 }, + preparation: Promise.resolve({ + revision: 1, + status: 'prepared' as const, + transaction: { ...transaction, transactionRevision: 1 }, + }), + }; + beginAtomicBatchUpdateMock.mockReturnValue(result); + return result; + } + + function getStateWithOldQuote(): TransactionPayControllerOptions['state'] { + return { + transactionData: { + [TRANSACTION_ID_MOCK]: { + fiatPayment: {}, + isLoading: false, + quotes: [ + { + strategy: TransactionPayStrategy.Relay, + } as TransactionPayQuote, + ], + quotesLastUpdated: 123, + tokens: [], + totals: {} as TransactionPayTotals, + }, + }, + }; + } + + function expectOldQuoteInvalidated( + controller: TransactionPayController, + isLoading: boolean, + ): void { + const transactionData = + controller.state.transactionData[TRANSACTION_ID_MOCK]; + + expect(transactionData.isLoading).toBe(isLoading); + expect(transactionData.quotes).toBeUndefined(); + expect(transactionData.quotesLastUpdated).toBeUndefined(); + expect(transactionData.totals).toBeUndefined(); + } + + it('passes the exact human amount and commits the complete patch once', async () => { + const prepareTransactionAmount = jest.fn().mockResolvedValue({ + kind: 'prepared', + amountRaw: '123456', + requiredAssets, + nestedTransactionUpdates, + requiredNestedTransactionIndexes: [0, 1], + }); + getTransactionMock.mockReturnValue(transaction); + mockAtomicUpdate(); + const controller = createController({ prepareTransactionAmount }); + controller.setTransactionConfig(TRANSACTION_ID_MOCK, () => undefined); + updateQuotesMock.mockClear(); + + expect( + await controller.updateAmount({ + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '1.23456', + }), + ).toBe(true); + + expect(prepareTransactionAmount).toHaveBeenCalledWith({ + amountHuman: '1.23456', + signal: expect.any(AbortSignal), + transaction, + }); + expect(beginAtomicBatchUpdateMock).toHaveBeenCalledWith({ + transactionId: TRANSACTION_ID_MOCK, + requiredAssets, + nestedTransactionUpdates, + }); + expect(updateQuotesMock).toHaveBeenCalledTimes(1); + expect(updateQuotesMock).toHaveBeenCalledWith( + expect.objectContaining({ + transactionRevision: 1, + transactionPreparation: expect.any(Promise), + }), + ); + }); + + it('invalidates an old quote before the preparation callback and keeps it cleared when the callback fails', async () => { + const callbackError = new Error('Amount callback failed'); + const controller = createController({ + prepareTransactionAmount: jest.fn().mockRejectedValue(callbackError), + state: getStateWithOldQuote(), + }); + getTransactionMock.mockReturnValue(transaction); + + const result = controller.updateAmount({ + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '1.25', + }); + + expectOldQuoteInvalidated(controller, true); + await expect(result).rejects.toThrow(callbackError); + expectOldQuoteInvalidated(controller, false); + expect(beginAtomicBatchUpdateMock).not.toHaveBeenCalled(); + expect(updateQuotesMock).not.toHaveBeenCalled(); + }); + + it('keeps an old quote cleared when revision preparation or vendor quoting fails', async () => { + const pipelineError = new Error('Quote pipeline failed'); + const controller = createController({ + prepareTransactionAmount: jest.fn().mockResolvedValue({ + kind: 'prepared', + amountRaw: '1250000', + requiredAssets, + nestedTransactionUpdates, + requiredNestedTransactionIndexes: [0, 1], + }), + state: getStateWithOldQuote(), + }); + getTransactionMock.mockReturnValue(transaction); + mockAtomicUpdate(); + updateQuotesMock.mockRejectedValue(pipelineError); + + const result = controller.updateAmount({ + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '1.25', + }); + + expectOldQuoteInvalidated(controller, true); + await expect(result).rejects.toThrow(pipelineError); + expectOldQuoteInvalidated(controller, false); + }); + + it('does not let a superseded amount generation clear loading for the current generation', async () => { + const firstPreparation = + createDeferredPromise(); + const secondPreparation = + createDeferredPromise(); + const prepareTransactionAmount = jest + .fn() + .mockReturnValueOnce(firstPreparation.promise) + .mockReturnValueOnce(secondPreparation.promise); + const controller = createController({ + prepareTransactionAmount, + state: getStateWithOldQuote(), + }); + getTransactionMock.mockReturnValue(transaction); + mockAtomicUpdate(); + + const first = controller.updateAmount({ + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '1', + }); + const second = controller.updateAmount({ + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '2', + }); + + firstPreparation.resolve({ kind: 'not-applicable' }); + expect(await first).toBe(false); + expectOldQuoteInvalidated(controller, true); + + secondPreparation.resolve({ + kind: 'prepared', + amountRaw: '2000000', + requiredAssets, + nestedTransactionUpdates, + requiredNestedTransactionIndexes: [0, 1], + }); + expect(await second).toBe(true); + expectOldQuoteInvalidated(controller, false); + }); + + it('joins an identical in-flight intent', async () => { + const preparation = createDeferredPromise<{ + kind: 'prepared'; + amountRaw: string; + requiredAssets: typeof requiredAssets; + nestedTransactionUpdates: typeof nestedTransactionUpdates; + requiredNestedTransactionIndexes: number[]; + }>(); + const prepareTransactionAmount = jest + .fn() + .mockReturnValue(preparation.promise); + getTransactionMock.mockReturnValue(transaction); + mockAtomicUpdate(); + const controller = createController({ prepareTransactionAmount }); + controller.setTransactionConfig(TRANSACTION_ID_MOCK, () => undefined); + const request = { + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '1.5', + }; + + const first = controller.updateAmount(request); + const second = controller.updateAmount(request); + + expect(first).toBe(second); + expect(prepareTransactionAmount).toHaveBeenCalledTimes(1); + + preparation.resolve({ + kind: 'prepared', + amountRaw: '1500000', + requiredAssets, + nestedTransactionUpdates, + requiredNestedTransactionIndexes: [0, 1], + }); + expect(await first).toBe(true); + }); + + it('aborts a different in-flight intent', async () => { + const firstPreparation = createDeferredPromise<{ + kind: 'not-applicable'; + }>(); + const signals: AbortSignal[] = []; + const prepareTransactionAmount = jest + .fn() + .mockImplementationOnce(({ signal }: { signal: AbortSignal }) => { + signals.push(signal); + return firstPreparation.promise; + }) + .mockResolvedValueOnce({ + kind: 'prepared', + amountRaw: '2000000', + requiredAssets, + nestedTransactionUpdates, + requiredNestedTransactionIndexes: [0, 1], + }); + getTransactionMock.mockReturnValue(transaction); + mockAtomicUpdate(); + const controller = createController({ prepareTransactionAmount }); + controller.setTransactionConfig(TRANSACTION_ID_MOCK, () => undefined); + + const first = controller.updateAmount({ + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '1', + }); + const second = controller.updateAmount({ + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '2', + }); + + expect(signals[0].aborted).toBe(true); + firstPreparation.resolve({ kind: 'not-applicable' }); + expect(await first).toBe(false); + expect(await second).toBe(true); + }); + + it('rejects a partial patch without committing a revision', async () => { + const controller = createController({ + prepareTransactionAmount: jest.fn().mockResolvedValue({ + kind: 'prepared', + amountRaw: '123', + requiredAssets, + nestedTransactionUpdates: [nestedTransactionUpdates[0]], + requiredNestedTransactionIndexes: [0, 1], + }), + }); + getTransactionMock.mockReturnValue(transaction); + + await expect( + controller.updateAmount({ + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '1.23', + }), + ).rejects.toThrow('incomplete patch'); + expect(beginAtomicBatchUpdateMock).not.toHaveBeenCalled(); + }); + + it('suppresses the listener quote launch caused by its atomic publication', async () => { + const prepareTransactionAmount = jest.fn().mockResolvedValue({ + kind: 'prepared', + amountRaw: '123456', + requiredAssets, + nestedTransactionUpdates, + requiredNestedTransactionIndexes: [0, 1], + }); + getTransactionMock.mockReturnValue(transaction); + const controller = createController({ prepareTransactionAmount }); + controller.setTransactionConfig(TRANSACTION_ID_MOCK, () => undefined); + updateQuotesMock.mockClear(); + const listenerUpdateTransactionData = + subscribeTransactionChangesMock.mock.calls[0][1]; + const atomicUpdate = mockAtomicUpdate(); + beginAtomicBatchUpdateMock.mockImplementationOnce((request) => { + listenerUpdateTransactionData(request.transactionId, (data) => { + data.tokens = [{ address: TOKEN_ADDRESS_MOCK }] as never; + }); + return atomicUpdate; + }); + + await controller.updateAmount({ + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '1.23456', + }); + + expect(updateQuotesMock).toHaveBeenCalledTimes(1); + }); + }); + describe('updatePaymentToken', () => { it('calls util', () => { createController().updatePaymentToken({ diff --git a/packages/transaction-pay-controller/src/TransactionPayController.ts b/packages/transaction-pay-controller/src/TransactionPayController.ts index 43794db9f30..ac2f23dace8 100644 --- a/packages/transaction-pay-controller/src/TransactionPayController.ts +++ b/packages/transaction-pay-controller/src/TransactionPayController.ts @@ -1,6 +1,9 @@ import type { StateMetadata } from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; -import type { TransactionMeta } from '@metamask/transaction-controller'; +import type { + BeginAtomicBatchUpdateResult, + TransactionMeta, +} from '@metamask/transaction-controller'; import type { Draft } from 'immer'; import { noop } from 'lodash'; @@ -17,12 +20,15 @@ import type { GetDelegationTransactionCallback, GetPaymentOverrideDataCallback, PolymarketCallbacks, + PrepareTransactionAmountCallback, + PrepareTransactionAmountResult, TransactionConfigCallback, TransactionData, TransactionPayControllerMessenger, TransactionPayFiatOptions, TransactionPayControllerOptions, TransactionPayControllerState, + UpdateAmountRequest, UpdateFiatPaymentRequest, UpdatePaymentTokenRequest, } from './types.js'; @@ -30,6 +36,7 @@ import { getStrategyOrder } from './utils/feature-flags.js'; import { updateQuotes } from './utils/quotes.js'; import { updateSourceAmounts } from './utils/source-amounts.js'; import { + getTransaction, subscribeAssetChanges, subscribeTransactionChanges, } from './utils/transaction.js'; @@ -43,6 +50,7 @@ const MESSENGER_EXPOSED_METHODS = [ 'polymarketGetDepositWalletAddress', 'polymarketSubmitDepositWalletBatch', 'setTransactionConfig', + 'updateAmount', 'updateFiatPayment', 'updatePaymentToken', ] as const; @@ -83,6 +91,19 @@ export class TransactionPayController extends BaseController< readonly #polymarket?: PolymarketCallbacks; + readonly #prepareTransactionAmount?: PrepareTransactionAmountCallback; + + readonly #quoteSuppressedTransactionIds = new Set(); + + readonly #amountUpdates = new Map< + string, + { + controller: AbortController; + intentKey: string; + promise: Promise; + } + >(); + constructor({ fiatOptions, getAmountData, @@ -92,6 +113,7 @@ export class TransactionPayController extends BaseController< getStrategies, messenger, polymarket, + prepareTransactionAmount, state, }: TransactionPayControllerOptions) { super({ @@ -108,6 +130,7 @@ export class TransactionPayController extends BaseController< this.#getStrategy = getStrategy; this.#getStrategies = getStrategies; this.#polymarket = polymarket; + this.#prepareTransactionAmount = prepareTransactionAmount; this.messenger.registerMethodActionHandlers( this, @@ -182,6 +205,152 @@ export class TransactionPayController extends BaseController< }); } + /** + * Prepares and atomically commits an exact transaction amount, then launches + * one quote generation joined to revision-bound local preparation. + * Identical in-flight intents share the same promise; different intents + * supersede and abort earlier work. + * + * @param request - Exact amount and transaction ID. + * @returns Whether the matching quote generation was published. + */ + updateAmount(request: UpdateAmountRequest): Promise { + const { amountHuman, transactionId } = request; + const intentKey = JSON.stringify({ amountHuman, transactionId }); + const existing = this.#amountUpdates.get(transactionId); + + if ( + existing?.intentKey === intentKey && + !existing.controller.signal.aborted + ) { + return existing.promise; + } + + existing?.controller.abort(new Error('Superseded by newer amount update')); + + const controller = new AbortController(); + const promise = this.#updateAmountInternal(request, controller.signal); + const trackedPromise = promise.finally(() => { + if (this.#amountUpdates.get(transactionId)?.promise === trackedPromise) { + this.#updateTransactionData(transactionId, (transactionData) => { + transactionData.isLoading = false; + }); + this.#amountUpdates.delete(transactionId); + } + }); + + this.#amountUpdates.set(transactionId, { + controller, + intentKey, + promise: trackedPromise, + }); + + return trackedPromise; + } + + async #updateAmountInternal( + { amountHuman, transactionId }: UpdateAmountRequest, + signal: AbortSignal, + ): Promise { + const transaction = getTransaction(transactionId, this.messenger); + + if (!transaction) { + throw new Error(`Transaction not found: ${transactionId}`); + } + + if (!this.#prepareTransactionAmount) { + throw new Error('Transaction amount preparation is not configured'); + } + + this.#updateTransactionData(transactionId, (transactionData) => { + transactionData.isLoading = true; + transactionData.quotes = undefined; + transactionData.quotesLastUpdated = undefined; + transactionData.totals = undefined; + }); + + const amountPreparation = await this.#prepareTransactionAmount({ + amountHuman, + signal, + transaction, + }); + + if (signal.aborted) { + return false; + } + + this.#validateAmountPreparation(amountPreparation); + + if (amountPreparation.kind === 'not-applicable') { + throw new Error('Transaction amount preparation is not applicable'); + } + + const atomicUpdate = this.#beginAtomicBatchUpdate( + transactionId, + amountPreparation, + ); + + return await updateQuotes({ + getStrategies: this.#getStrategiesWithFallback.bind(this), + messenger: this.messenger, + signal, + transactionData: this.state.transactionData[transactionId], + transactionId, + transactionPreparation: atomicUpdate.preparation, + transactionRevision: atomicUpdate.revision, + updateTransactionData: this.#updateTransactionData.bind(this), + }); + } + + #beginAtomicBatchUpdate( + transactionId: string, + amountPreparation: Extract< + PrepareTransactionAmountResult, + { kind: 'prepared' } + >, + ): BeginAtomicBatchUpdateResult { + this.#quoteSuppressedTransactionIds.add(transactionId); + + try { + return this.messenger.call( + 'TransactionController:beginAtomicBatchUpdate', + { + transactionId, + requiredAssets: amountPreparation.requiredAssets, + nestedTransactionUpdates: amountPreparation.nestedTransactionUpdates, + }, + ); + } finally { + this.#quoteSuppressedTransactionIds.delete(transactionId); + } + } + + #validateAmountPreparation(result: PrepareTransactionAmountResult): void { + if (result.kind === 'not-applicable') { + return; + } + + const requiredIndexes = new Set(result.requiredNestedTransactionIndexes); + const updateIndexes = new Set( + result.nestedTransactionUpdates.map( + ({ transactionIndex }) => transactionIndex, + ), + ); + + const hasCompletePatch = + requiredIndexes.size > 0 && + requiredIndexes.size === result.requiredNestedTransactionIndexes.length && + updateIndexes.size === result.nestedTransactionUpdates.length && + requiredIndexes.size === updateIndexes.size && + [...requiredIndexes].every((index) => updateIndexes.has(index)); + + if (!hasCompletePatch) { + throw new Error( + 'Transaction amount preparation returned an incomplete patch', + ); + } + } + /** * Updates the payment token for a transaction. * @@ -385,7 +554,10 @@ export class TransactionPayController extends BaseController< } }); - if (shouldUpdateQuotes) { + if ( + shouldUpdateQuotes && + !this.#quoteSuppressedTransactionIds.has(transactionId) + ) { updateQuotes({ getStrategies: this.#getStrategiesWithFallback.bind(this), messenger: this.messenger, diff --git a/packages/transaction-pay-controller/src/index.ts b/packages/transaction-pay-controller/src/index.ts index d8bf3f91c1e..dbda9559862 100644 --- a/packages/transaction-pay-controller/src/index.ts +++ b/packages/transaction-pay-controller/src/index.ts @@ -4,6 +4,9 @@ export type { GetAmountDataResponse, GetPaymentOverrideDataRequest, GetPaymentOverrideDataResponse, + PrepareTransactionAmountCallback, + PrepareTransactionAmountRequest, + PrepareTransactionAmountResult, TransactionConfig, TransactionConfigCallback, TransactionData, @@ -25,6 +28,7 @@ export type { TransactionPayRequiredToken, TransactionPaySourceAmount, TransactionPayTotals, + UpdateAmountRequest, UpdateFiatPaymentRequest, UpdatePaymentTokenRequest, } from './types.js'; @@ -36,6 +40,7 @@ export type { TransactionPayControllerPolymarketGetDepositWalletAddressAction, TransactionPayControllerPolymarketSubmitDepositWalletBatchAction, TransactionPayControllerSetTransactionConfigAction, + TransactionPayControllerUpdateAmountAction, TransactionPayControllerUpdatePaymentTokenAction, TransactionPayControllerUpdateFiatPaymentAction, } from './TransactionPayController-method-action-types.js'; diff --git a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts index 0bf88e00f27..65a69f2e166 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts @@ -1,10 +1,12 @@ import { toHex } from '@metamask/controller-utils'; import { TransactionType } from '@metamask/transaction-controller'; import type { + AtomicBatchPreparationResult, GasFeeToken, TransactionMeta, } from '@metamask/transaction-controller'; import type { Hex } from '@metamask/utils'; +import { createDeferredPromise } from '@metamask/utils'; import { cloneDeep } from 'lodash'; import { getDefaultRemoteFeatureFlagControllerState } from '../../../../remote-feature-flag-controller/src/remote-feature-flag-controller.js'; @@ -246,6 +248,40 @@ describe('Relay Quotes Utils', () => { }); describe('getRelayQuotes', () => { + it('fetches the raw standard quote before preparation and normalizes afterward', async () => { + successfulFetchMock.mockResolvedValue({ + ok: true, + json: async () => QUOTE_MOCK, + } as never); + const transactionPreparation = + createDeferredPromise(); + + const resultPromise = getRelayQuotes({ + accountSupports7702: true, + messenger, + requests: [QUOTE_REQUEST_MOCK], + transaction: TRANSACTION_META_MOCK, + transactionPreparation: transactionPreparation.promise, + }); + + await new Promise((resolve) => process.nextTick(resolve)); + + expect(successfulFetchMock).toHaveBeenCalledTimes(1); + expect(calculateGasCostMock).not.toHaveBeenCalled(); + + transactionPreparation.resolve({ + revision: 1, + status: 'prepared', + transaction: { + ...TRANSACTION_META_MOCK, + transactionRevision: 1, + }, + }); + + expect(await resultPromise).toHaveLength(1); + expect(calculateGasCostMock).toHaveBeenCalled(); + }); + it('returns quotes from Relay', async () => { successfulFetchMock.mockResolvedValue({ ok: true, diff --git a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts index 6afdb789009..124a8f0cef9 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts @@ -365,7 +365,26 @@ async function getSingleQuote( log('Fetched relay quote', quote); - return await normalizeQuote(quote, request, fullRequest); + let normalizationRequest = fullRequest; + const transactionPreparation = + !request.isMaxAmount && !request.isPostQuote + ? fullRequest.transactionPreparation + : undefined; + + if (transactionPreparation) { + const preparationResult = await transactionPreparation; + + if (preparationResult.status !== 'prepared') { + throw new Error('Transaction preparation was superseded'); + } + + normalizationRequest = { + ...fullRequest, + transaction: preparationResult.transaction, + }; + } + + return await normalizeQuote(quote, request, normalizationRequest); } catch (error) { log('Error fetching relay quote', error); throw error; diff --git a/packages/transaction-pay-controller/src/tests/messenger-mock.ts b/packages/transaction-pay-controller/src/tests/messenger-mock.ts index f81bbf7516d..f485c8a2eb6 100644 --- a/packages/transaction-pay-controller/src/tests/messenger-mock.ts +++ b/packages/transaction-pay-controller/src/tests/messenger-mock.ts @@ -17,6 +17,7 @@ import type { SentinelApiServiceSimulateTransactionsAction } from '@metamask/sen import type { TransactionControllerAddTransactionAction, TransactionControllerAddTransactionBatchAction, + TransactionControllerBeginAtomicBatchUpdateAction, TransactionControllerEstimateGasAction, TransactionControllerEstimateGasBatchAction, TransactionControllerGetGasFeeTokensAction, @@ -70,6 +71,10 @@ export function getMessengerMock({ TransactionControllerAddTransactionBatchAction['handler'] > = jest.fn(); + const beginAtomicBatchUpdateMock: jest.MockedFn< + TransactionControllerBeginAtomicBatchUpdateAction['handler'] + > = jest.fn(); + const findNetworkClientIdByChainIdMock: jest.MockedFn< NetworkControllerFindNetworkClientIdByChainIdAction['handler'] > = jest.fn(); @@ -297,6 +302,11 @@ export function getMessengerMock({ ); } + messenger.registerActionHandler( + 'TransactionController:beginAtomicBatchUpdate', + beginAtomicBatchUpdateMock, + ); + messenger.registerActionHandler( 'KeyringController:getState', getKeyringControllerStateMock, @@ -306,6 +316,7 @@ export function getMessengerMock({ return { addTransactionMock, + beginAtomicBatchUpdateMock, getAssetsControllerStateMock, addTransactionBatchMock, estimateGasMock, diff --git a/packages/transaction-pay-controller/src/types.ts b/packages/transaction-pay-controller/src/types.ts index 0c25e972cc7..c942f1b6360 100644 --- a/packages/transaction-pay-controller/src/types.ts +++ b/packages/transaction-pay-controller/src/types.ts @@ -35,8 +35,12 @@ import type { import type { RemoteFeatureFlagControllerGetStateAction } from '@metamask/remote-feature-flag-controller'; import type { SentinelApiServiceActions } from '@metamask/sentinel-api-service'; import type { + AtomicBatchPreparationResult, AuthorizationList, + NestedTransactionUpdate, + RequiredAsset, TransactionControllerAddTransactionBatchAction, + TransactionControllerBeginAtomicBatchUpdateAction, TransactionControllerEstimateGasAction, TransactionControllerEstimateGasBatchAction, TransactionControllerUnapprovedTransactionAddedEvent, @@ -80,6 +84,7 @@ export type AllowedActions = | SentinelApiServiceActions | TransactionControllerAddTransactionAction | TransactionControllerAddTransactionBatchAction + | TransactionControllerBeginAtomicBatchUpdateAction | TransactionControllerEstimateGasAction | TransactionControllerEstimateGasBatchAction | TransactionControllerGetGasFeeTokensAction @@ -203,6 +208,55 @@ export type GetAmountDataCallback = ( request: GetAmountDataRequest, ) => Promise; +/** Request passed to the explicit amount preparation callback. */ +export type PrepareTransactionAmountRequest = { + /** Exact human-readable decimal amount selected by the caller. */ + amountHuman: string; + + /** Signal aborted when a different amount intent supersedes this request. */ + signal: AbortSignal; + + /** Coherent transaction snapshot to prepare. */ + transaction: TransactionMeta; +}; + +/** Result returned by the explicit amount preparation callback. */ +export type PrepareTransactionAmountResult = + | { + /** Indicates that this transaction adopts explicit amount preparation. */ + kind: 'prepared'; + + /** Raw atomic-unit amount corresponding to `amountHuman`. */ + amountRaw: string; + + /** Complete assets required by the prepared transaction. */ + requiredAssets: RequiredAsset[]; + + /** Complete nested calldata patch. */ + nestedTransactionUpdates: NestedTransactionUpdate[]; + + /** Exact indexes that must be present in the nested calldata patch. */ + requiredNestedTransactionIndexes: number[]; + } + | { + /** Indicates that explicit amount preparation does not apply. */ + kind: 'not-applicable'; + }; + +/** Callback that prepares a complete transaction patch for an exact amount. */ +export type PrepareTransactionAmountCallback = ( + request: PrepareTransactionAmountRequest, +) => Promise; + +/** Request to explicitly update a transaction amount. */ +export type UpdateAmountRequest = { + /** Exact human-readable decimal amount selected by the caller. */ + amountHuman: string; + + /** ID of the transaction to update. */ + transactionId: string; +}; + /** Callback to update fiat payment state. */ export type TransactionFiatPaymentCallback = ( fiatPayment: TransactionFiatPayment, @@ -242,6 +296,9 @@ export type TransactionPayControllerOptions = { /** Optional callback to re-encode nested transaction calldata for a given amount. */ getAmountData?: GetAmountDataCallback; + /** Optional callback used by the explicit amount update proof of concept. */ + prepareTransactionAmount?: PrepareTransactionAmountCallback; + /** Callback to convert a transaction into a redeem delegation. */ getDelegationTransaction: GetDelegationTransactionCallback; @@ -665,6 +722,9 @@ export type PayStrategyGetQuotesRequest = { /** Metadata of the original target transaction. */ transaction: TransactionMeta; + + /** Revision-bound local preparation for an explicit amount update. */ + transactionPreparation?: Promise; }; /** Request to submit quotes for a transaction. */ diff --git a/packages/transaction-pay-controller/src/utils/quotes.test.ts b/packages/transaction-pay-controller/src/utils/quotes.test.ts index da84b82a549..60f34ba06d3 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.test.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.test.ts @@ -1,7 +1,11 @@ import { TransactionStatus } from '@metamask/transaction-controller'; -import type { TransactionMeta } from '@metamask/transaction-controller'; +import type { + AtomicBatchPreparationResult, + TransactionMeta, +} from '@metamask/transaction-controller'; import type { BatchTransaction } from '@metamask/transaction-controller'; import type { Hex, Json } from '@metamask/utils'; +import { createDeferredPromise } from '@metamask/utils'; import { cloneDeep } from 'lodash'; import { TransactionPayStrategy } from '../constants.js'; @@ -200,6 +204,40 @@ describe('Quotes Utils', () => { }); describe('updateQuotes', () => { + it('does not publish an executable quote until matching preparation completes', async () => { + const transactionPreparation = + createDeferredPromise(); + const preparedTransaction = { + ...TRANSACTION_META_MOCK, + transactionRevision: 1, + }; + getTransactionMock.mockReturnValue(preparedTransaction); + + const resultPromise = run({ + transactionPreparation: transactionPreparation.promise, + transactionRevision: 1, + }); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + expect(getQuotesMock).toHaveBeenCalled(); + expect(calculateTotalsMock).not.toHaveBeenCalled(); + + transactionPreparation.resolve({ + revision: 1, + status: 'prepared', + transaction: preparedTransaction, + }); + + expect(await resultPromise).toBe(true); + expect(calculateTotalsMock).toHaveBeenCalledWith( + expect.objectContaining({ transaction: preparedTransaction }), + ); + }); + it('updates quotes in state', async () => { await run(); diff --git a/packages/transaction-pay-controller/src/utils/quotes.ts b/packages/transaction-pay-controller/src/utils/quotes.ts index 515b0734daa..d707b0acc97 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.ts @@ -1,6 +1,9 @@ import { TransactionStatus } from '@metamask/transaction-controller'; -import type { BatchTransaction } from '@metamask/transaction-controller'; -import type { TransactionMeta } from '@metamask/transaction-controller'; +import type { + AtomicBatchPreparationResult, + BatchTransaction, + TransactionMeta, +} from '@metamask/transaction-controller'; import type { Hex, Json } from '@metamask/utils'; import { createModuleLogger } from '@metamask/utils'; @@ -44,8 +47,11 @@ const inFlightQuoteRequests = new Map(); export type UpdateQuotesRequest = { getStrategies: (transaction: TransactionMeta) => TransactionPayStrategy[]; messenger: TransactionPayControllerMessenger; + signal?: AbortSignal; transactionData: TransactionData | undefined; transactionId: string; + transactionPreparation?: Promise; + transactionRevision?: number; updateTransactionData: UpdateTransactionDataCallback; }; @@ -66,8 +72,11 @@ export async function updateQuotes( const { getStrategies, messenger, + signal: externalSignal, transactionData, transactionId, + transactionPreparation, + transactionRevision, updateTransactionData, } = request; @@ -102,6 +111,16 @@ export async function updateQuotes( const controller = abortPreviousAndCreateController(transactionId); const { signal } = controller; + const abortFromExternalSignal = (): void => + controller.abort(externalSignal?.reason); + + if (externalSignal?.aborted) { + abortFromExternalSignal(); + } else { + externalSignal?.addEventListener('abort', abortFromExternalSignal, { + once: true, + }); + } updateTransactionData(transactionId, (data) => { data.isLoading = true; @@ -150,6 +169,7 @@ export async function updateQuotes( messenger, fiatPayment?.selectedPaymentMethodId, signal, + transactionPreparation, ); if (signal.aborted) { @@ -157,6 +177,31 @@ export async function updateQuotes( return false; } + let preparedTransaction = transaction; + + if (transactionPreparation) { + const preparationResult = await transactionPreparation; + const latestTransaction = getTransaction(transactionId, messenger); + + if ( + preparationResult.status !== 'prepared' || + preparationResult.revision !== transactionRevision || + latestTransaction?.transactionRevision !== transactionRevision + ) { + log('Discarding quotes for stale transaction revision', { + transactionId, + transactionRevision, + }); + return false; + } + + preparedTransaction = preparationResult.transaction; + } + + if (signal.aborted) { + return false; + } + // No-op quotes mark direct routes. They have no fees or amounts and the // transaction is signed and submitted locally, so totals and transaction // sync must treat them as "no quotes". @@ -170,7 +215,7 @@ export async function updateQuotes( messenger, quotes: executableQuotes as TransactionPayQuote[], tokens, - transaction, + transaction: preparedTransaction, }); log('Calculated totals', { transactionId, totals }); @@ -204,6 +249,7 @@ export async function updateQuotes( data.isLoading = false; }); } + externalSignal?.removeEventListener('abort', abortFromExternalSignal); clearControllerIfCurrent(transactionId, controller); } @@ -619,6 +665,7 @@ async function refreshPaymentTokenBalance({ * @param messenger - Controller messenger. * @param fiatPaymentMethod - Selected fiat payment method ID, if applicable. * @param signal - Signal that aborts when the quote request is superseded. + * @param transactionPreparation - Revision-bound local preparation. * @returns An object containing batch transactions and quotes. */ async function getQuotes( @@ -632,6 +679,7 @@ async function getQuotes( messenger: TransactionPayControllerMessenger, fiatPaymentMethod?: string, signal?: AbortSignal, + transactionPreparation?: Promise, ): Promise<{ batchTransactions: BatchTransaction[]; error?: QuoteErrorInfo; @@ -677,6 +725,7 @@ async function getQuotes( requests, signal, transaction, + transactionPreparation, }; let error: QuoteErrorInfo | undefined; From 782ee764abe3b70c7139af6b68beab68b49cd0fb Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 16 Jul 2026 17:09:29 +0100 Subject: [PATCH 02/11] docs: link controller changelogs to pull request --- packages/transaction-controller/CHANGELOG.md | 2 +- packages/transaction-pay-controller/CHANGELOG.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index d7ff2f13171..3f11c62f59c 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -31,7 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Web3Provider` schedules its JSON-RPC dispatch with `setTimeout`, which never fires on React Native iOS when the timer pump is starved, blocking `addTransaction` indefinitely and preventing dapp confirmations from appearing ([MetaMask/metamask-mobile#32863](https://github.com/MetaMask/metamask-mobile/issues/32863)) ### Added -- Add `beginAtomicBatchUpdate` for coherent multi-call updates with monotonic revisions and revision-bound gas preparation +- Add `beginAtomicBatchUpdate` for coherent multi-call updates with monotonic revisions and revision-bound gas preparation ([#9543](https://github.com/MetaMask/core/pull/9543)) ## [69.0.0] diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index 06f56f4660a..52037dbdfc4 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -51,7 +51,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/assets-controller` from `^11.0.0` to `^11.1.0` ([#9579](https://github.com/MetaMask/core/pull/9579)) ### Added -- Add explicit `updateAmount` orchestration with complete-patch validation, in-flight intent deduplication, and revision-bound Relay quote publication +- Add explicit `updateAmount` orchestration with complete-patch validation, in-flight intent deduplication, and revision-bound Relay quote publication ([#9543](https://github.com/MetaMask/core/pull/9543)) ## [25.0.0] From 3d14200c152d5c3e8aa00e12287bfcac3f636b6c Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 16 Jul 2026 17:48:20 +0100 Subject: [PATCH 03/11] test(transaction-pay-controller): cover stale quote guards --- .../src/TransactionPayController.test.ts | 44 +++++++ .../src/strategy/relay/relay-quotes.test.ts | 24 ++++ .../src/utils/quotes.test.ts | 113 ++++++++++++++++++ 3 files changed, 181 insertions(+) diff --git a/packages/transaction-pay-controller/src/TransactionPayController.test.ts b/packages/transaction-pay-controller/src/TransactionPayController.test.ts index 012f0ad6769..9c31feb7075 100644 --- a/packages/transaction-pay-controller/src/TransactionPayController.test.ts +++ b/packages/transaction-pay-controller/src/TransactionPayController.test.ts @@ -182,6 +182,50 @@ describe('TransactionPayController', () => { expect(transactionData.totals).toBeUndefined(); } + it('rejects an update for an unknown transaction', async () => { + getTransactionMock.mockReturnValue(undefined); + const controller = createController({ + prepareTransactionAmount: jest.fn(), + }); + + await expect( + controller.updateAmount({ + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '1.23', + }), + ).rejects.toThrow(`Transaction not found: ${TRANSACTION_ID_MOCK}`); + }); + + it('rejects an update when amount preparation is not configured', async () => { + getTransactionMock.mockReturnValue(transaction); + const controller = createController(); + + await expect( + controller.updateAmount({ + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '1.23', + }), + ).rejects.toThrow('Transaction amount preparation is not configured'); + }); + + it('rejects a non-applicable amount preparation', async () => { + getTransactionMock.mockReturnValue(transaction); + const controller = createController({ + prepareTransactionAmount: jest.fn().mockResolvedValue({ + kind: 'not-applicable', + }), + }); + updateQuotesMock.mockClear(); + + await expect( + controller.updateAmount({ + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '1.23', + }), + ).rejects.toThrow('Transaction amount preparation is not applicable'); + expect(beginAtomicBatchUpdateMock).not.toHaveBeenCalled(); + }); + it('passes the exact human amount and commits the complete patch once', async () => { const prepareTransactionAmount = jest.fn().mockResolvedValue({ kind: 'prepared', diff --git a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts index 65a69f2e166..ccf1a189e2b 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts @@ -282,6 +282,30 @@ describe('Relay Quotes Utils', () => { expect(calculateGasCostMock).toHaveBeenCalled(); }); + it('rejects a raw standard quote when transaction preparation was superseded', async () => { + successfulFetchMock.mockResolvedValue({ + ok: true, + json: async () => QUOTE_MOCK, + } as never); + + await expect( + getRelayQuotes({ + accountSupports7702: true, + messenger, + requests: [QUOTE_REQUEST_MOCK], + transaction: TRANSACTION_META_MOCK, + transactionPreparation: Promise.resolve({ + revision: 1, + status: 'superseded', + transaction: TRANSACTION_META_MOCK, + }), + }), + ).rejects.toThrow('Transaction preparation was superseded'); + + expect(successfulFetchMock).toHaveBeenCalledTimes(1); + expect(calculateGasCostMock).not.toHaveBeenCalled(); + }); + it('returns quotes from Relay', async () => { successfulFetchMock.mockResolvedValue({ ok: true, diff --git a/packages/transaction-pay-controller/src/utils/quotes.test.ts b/packages/transaction-pay-controller/src/utils/quotes.test.ts index 60f34ba06d3..faa5e20e42f 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.test.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.test.ts @@ -238,6 +238,119 @@ describe('Quotes Utils', () => { ); }); + it('discards quotes when preparation was superseded', async () => { + getTransactionMock.mockReturnValue({ + ...TRANSACTION_META_MOCK, + transactionRevision: 1, + }); + + const result = await run({ + transactionPreparation: Promise.resolve({ + revision: 1, + status: 'superseded', + transaction: TRANSACTION_META_MOCK, + }), + transactionRevision: 1, + }); + + expect(result).toBe(false); + expect(calculateTotalsMock).not.toHaveBeenCalled(); + }); + + it('discards quotes when preparation has a different revision', async () => { + getTransactionMock.mockReturnValue({ + ...TRANSACTION_META_MOCK, + transactionRevision: 1, + }); + + const result = await run({ + transactionPreparation: Promise.resolve({ + revision: 2, + status: 'prepared', + transaction: TRANSACTION_META_MOCK, + }), + transactionRevision: 1, + }); + + expect(result).toBe(false); + expect(calculateTotalsMock).not.toHaveBeenCalled(); + }); + + it('discards quotes when the latest transaction has a different revision', async () => { + getTransactionMock.mockReturnValue({ + ...TRANSACTION_META_MOCK, + transactionRevision: 2, + }); + + const result = await run({ + transactionPreparation: Promise.resolve({ + revision: 1, + status: 'prepared', + transaction: TRANSACTION_META_MOCK, + }), + transactionRevision: 1, + }); + + expect(result).toBe(false); + expect(calculateTotalsMock).not.toHaveBeenCalled(); + }); + + it('aborts immediately when the external signal is already aborted', async () => { + const externalController = new AbortController(); + externalController.abort(); + + const result = await run({ + signal: externalController.signal, + }); + + expect(result).toBe(false); + expect(calculateTotalsMock).not.toHaveBeenCalled(); + }); + + it('discards prepared quotes if the external signal aborts while preparation is pending', async () => { + const externalController = new AbortController(); + const transactionPreparation = + createDeferredPromise(); + const preparedTransaction = { + ...TRANSACTION_META_MOCK, + transactionRevision: 1, + }; + getTransactionMock.mockReturnValue(preparedTransaction); + + const resultPromise = run({ + signal: externalController.signal, + transactionPreparation: transactionPreparation.promise, + transactionRevision: 1, + }); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + expect(getQuotesMock).toHaveBeenCalled(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + externalController.abort(); + transactionPreparation.resolve({ + revision: 1, + status: 'prepared', + transaction: preparedTransaction, + }); + + expect(await resultPromise).toBe(false); + expect(calculateTotalsMock).not.toHaveBeenCalled(); + }); + it('updates quotes in state', async () => { await run(); From e2adea1ee64aa72a6a6ec090893b2414fd4f4342 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Wed, 22 Jul 2026 11:56:36 +0100 Subject: [PATCH 04/11] fix: preserve gas update after simulation failure --- .../src/TransactionController.test.ts | 22 +++++++++++++++++++ .../src/TransactionController.ts | 21 +++++++++++++----- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 85d38e5b13b..54aa4439b3f 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -7776,6 +7776,28 @@ describe('TransactionController', () => { await result.preparation; }); + it('commits successful gas preparation before propagating a simulation failure', async () => { + const simulationError = new Error('Simulation failed'); + updateGasMock.mockImplementationOnce(async ({ txMeta }) => { + txMeta.txParams.gas = '0x123'; + txMeta.gasLimitNoBuffer = '0x100'; + }); + getGasFeeTokensMock.mockRejectedValueOnce(simulationError); + const { controller } = setupAtomicBatchController(); + + const { preparation } = controller.beginAtomicBatchUpdate({ + transactionId: TRANSACTION_META_MOCK.id, + requiredAssets, + nestedTransactionUpdates: [ + { transactionIndex: 0, transactionData: '0xAAAA' }, + ], + }); + + await expect(preparation).rejects.toThrow(simulationError); + expect(controller.state.transactions[0].txParams.gas).toBe('0x123'); + expect(controller.state.transactions[0].gasLimitNoBuffer).toBe('0x100'); + }); + it('ignores simulation results started before the current revision', async () => { const staleSimulation = createDeferredPromise<{ simulationData: SimulationData; diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 5acf1980697..d5687226221 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -2720,12 +2720,17 @@ export class TransactionController extends BaseController< draftTransaction: TransactionMeta, revision: number, ): Promise { - await Promise.all([ - this.#updateGasEstimate(draftTransaction), - this.#isSimulationEnabled() - ? this.#updateSimulationData(draftTransaction) - : Promise.resolve(), - ]); + const [gasPreparationResult, simulationPreparationResult] = + await Promise.allSettled([ + this.#updateGasEstimate(draftTransaction), + this.#isSimulationEnabled() + ? this.#updateSimulationData(draftTransaction) + : Promise.resolve(), + ]); + + if (gasPreparationResult.status === 'rejected') { + throw gasPreparationResult.reason; + } const currentTransaction = this.#getTransaction(draftTransaction.id); @@ -2757,6 +2762,10 @@ export class TransactionController extends BaseController< }, ); + if (simulationPreparationResult.status === 'rejected') { + throw simulationPreparationResult.reason; + } + return { revision, status: 'prepared', From 472db143ab4cace0d57f36ac75d4b3254b91ee2e Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Wed, 22 Jul 2026 12:06:48 +0100 Subject: [PATCH 05/11] refactor: extract EIP-7702 batch update utility --- packages/transaction-controller/CHANGELOG.md | 2 +- .../src/TransactionController.ts | 50 +++----------- packages/transaction-controller/src/index.ts | 1 + .../src/utils/eip7702.test.ts | 66 +++++++++++++++++++ .../src/utils/eip7702.ts | 54 +++++++++++++++ 5 files changed, 130 insertions(+), 43 deletions(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 3f11c62f59c..cafbdf43965 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -31,7 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Web3Provider` schedules its JSON-RPC dispatch with `setTimeout`, which never fires on React Native iOS when the timer pump is starved, blocking `addTransaction` indefinitely and preventing dapp confirmations from appearing ([MetaMask/metamask-mobile#32863](https://github.com/MetaMask/metamask-mobile/issues/32863)) ### Added -- Add `beginAtomicBatchUpdate` for coherent multi-call updates with monotonic revisions and revision-bound gas preparation ([#9543](https://github.com/MetaMask/core/pull/9543)) +- Add `beginAtomicBatchUpdate` for coherent multi-call updates with monotonic revisions and revision-bound gas preparation, and export `updateEIP7702BatchData` for synchronous indexed updates to nested transaction calldata ([#9543](https://github.com/MetaMask/core/pull/9543)) ## [69.0.0] diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index d5687226221..749836c1b61 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -144,9 +144,9 @@ import { import { getBalanceChanges } from './utils/balance-changes.js'; import { addTransactionBatch, isAtomicBatchSupported } from './utils/batch.js'; import { - generateEIP7702BatchTransaction, getDelegationAddress, signAuthorizationList, + updateEIP7702BatchData, } from './utils/eip7702.js'; import { validateConfirmedExternalTransaction } from './utils/external-transactions.js'; import { @@ -2586,56 +2586,22 @@ export class TransactionController extends BaseController< ); } - const updateIndexes = new Set(); - - for (const { transactionIndex } of nestedTransactionUpdates) { - if (updateIndexes.has(transactionIndex)) { - throw new Error( - `Duplicate nested transaction index - ${transactionIndex}`, - ); - } - - if (!currentTransaction.nestedTransactions?.[transactionIndex]) { - throw new Error( - `Nested transaction not found with index - ${transactionIndex}`, - ); - } - - updateIndexes.add(transactionIndex); - } + const { nestedTransactions, transactionData } = updateEIP7702BatchData( + currentTransaction.txParams.from as Hex, + currentTransaction.nestedTransactions ?? [], + nestedTransactionUpdates, + ); log('Beginning atomic batch update', request); const updatedTransactionMeta = this.#updateTransactionInternal( { transactionId, skipResimulateCheck: true }, (transactionMeta) => { - const { nestedTransactions, txParams } = transactionMeta; - const from = txParams.from as Hex; - - for (const { - transactionIndex, - transactionData, - } of nestedTransactionUpdates) { - const nestedTransaction = nestedTransactions?.[transactionIndex]; - - if (!nestedTransaction) { - throw new Error( - `Nested transaction not found with index - ${transactionIndex}`, - ); - } - - nestedTransaction.data = transactionData; - } - - const batchTransaction = generateEIP7702BatchTransaction( - from, - nestedTransactions ?? [], - ); - revision = (transactionMeta.transactionRevision ?? 0) + 1; + transactionMeta.nestedTransactions = nestedTransactions; transactionMeta.requiredAssets = requiredAssets; transactionMeta.transactionRevision = revision; - transactionMeta.txParams.data = batchTransaction.data; + transactionMeta.txParams.data = transactionData; transactionMeta.txParams.gas = undefined; transactionMeta.gasLimitNoBuffer = undefined; transactionMeta.gasUsed = undefined; diff --git a/packages/transaction-controller/src/index.ts b/packages/transaction-controller/src/index.ts index 8034d9f11c7..ffd3efdff6b 100644 --- a/packages/transaction-controller/src/index.ts +++ b/packages/transaction-controller/src/index.ts @@ -137,6 +137,7 @@ export { mergeGasFeeEstimates } from './utils/gas-flow.js'; export { decodeAuthorizationSignature, generateEIP7702BatchTransaction, + updateEIP7702BatchData, } from './utils/eip7702.js'; export { isEIP1559Transaction, diff --git a/packages/transaction-controller/src/utils/eip7702.test.ts b/packages/transaction-controller/src/utils/eip7702.test.ts index 2d28c16e94d..dca0a1fb050 100644 --- a/packages/transaction-controller/src/utils/eip7702.test.ts +++ b/packages/transaction-controller/src/utils/eip7702.test.ts @@ -25,6 +25,7 @@ import { getDelegationAddress, isAccountUpgradedToEIP7702, signAuthorizationList, + updateEIP7702BatchData, } from './eip7702.js'; import { getEIP7702ContractAddresses, @@ -609,6 +610,71 @@ describe('EIP-7702 Utils', () => { }); }); + describe('updateEIP7702BatchData', () => { + it('returns updated nested transactions and regenerated batch data without mutating the input', () => { + const nestedTransactions = [ + { + data: '0xaaaa' as Hex, + to: ADDRESS_2_MOCK as Hex, + value: '0x5678' as Hex, + }, + { + data: '0xbbbb' as Hex, + to: ADDRESS_3_MOCK as Hex, + value: '0xdef0' as Hex, + }, + ]; + + const result = updateEIP7702BatchData(ADDRESS_MOCK, nestedTransactions, [ + { transactionIndex: 0, transactionData: '0x1234' }, + { transactionIndex: 1, transactionData: '0x9abc' }, + ]); + + expect(result).toStrictEqual({ + nestedTransactions: [ + { + data: '0x1234', + to: ADDRESS_2_MOCK, + value: '0x5678', + }, + { + data: '0x9abc', + to: ADDRESS_3_MOCK, + value: '0xdef0', + }, + ], + transactionData: DATA_MOCK, + }); + expect(nestedTransactions.map(({ data }) => data)).toStrictEqual([ + '0xaaaa', + '0xbbbb', + ]); + }); + + it('throws if an update index is duplicated', () => { + expect(() => + updateEIP7702BatchData( + ADDRESS_MOCK, + [{ data: '0xaaaa' }], + [ + { transactionIndex: 0, transactionData: '0x1234' }, + { transactionIndex: 0, transactionData: '0x5678' }, + ], + ), + ).toThrow('Duplicate nested transaction index - 0'); + }); + + it('throws if an update index does not exist', () => { + expect(() => + updateEIP7702BatchData( + ADDRESS_MOCK, + [{ data: '0xaaaa' }], + [{ transactionIndex: 1, transactionData: '0x1234' }], + ), + ).toThrow('Nested transaction not found with index - 1'); + }); + }); + describe('generateEIP7702BatchTransaction', () => { it('generates a batch transaction', () => { const result = generateEIP7702BatchTransaction(ADDRESS_MOCK, [ diff --git a/packages/transaction-controller/src/utils/eip7702.ts b/packages/transaction-controller/src/utils/eip7702.ts index ea7f03792ac..25d47df010b 100644 --- a/packages/transaction-controller/src/utils/eip7702.ts +++ b/packages/transaction-controller/src/utils/eip7702.ts @@ -10,6 +10,7 @@ import { projectLogger } from '../logger.js'; import type { TransactionControllerMessenger } from '../TransactionController.js'; import type { BatchTransactionParams, + NestedTransactionUpdate, Authorization, AuthorizationList, TransactionMeta, @@ -164,6 +165,59 @@ export async function isAccountUpgradedToEIP7702( }; } +/** + * Update indexed transactions in an EIP-7702 batch and regenerate its calldata. + * + * @param from - The sender address. + * @param transactions - The existing nested transactions. + * @param updates - Indexed calldata updates. + * @returns Updated nested transactions and regenerated batch calldata. + */ +export function updateEIP7702BatchData( + from: Hex, + transactions: BatchTransactionParams[], + updates: NestedTransactionUpdate[], +): { + nestedTransactions: BatchTransactionParams[]; + transactionData: Hex; +} { + const updatesByIndex = new Map(); + + for (const { transactionIndex, transactionData } of updates) { + if (updatesByIndex.has(transactionIndex)) { + throw new Error( + `Duplicate nested transaction index - ${transactionIndex}`, + ); + } + + if (!transactions[transactionIndex]) { + throw new Error( + `Nested transaction not found with index - ${transactionIndex}`, + ); + } + + updatesByIndex.set(transactionIndex, transactionData); + } + + const nestedTransactions = transactions.map((transaction, index) => { + const transactionData = updatesByIndex.get(index); + + return { + ...transaction, + ...(transactionData === undefined ? {} : { data: transactionData }), + }; + }); + const batchTransaction = generateEIP7702BatchTransaction( + from, + nestedTransactions, + ); + + return { + nestedTransactions, + transactionData: batchTransaction.data as Hex, + }; +} + /** * Generate an EIP-7702 batch transaction. * From 8a2a85ade57e608cd8540c08302c7e4341768b7b Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Wed, 22 Jul 2026 12:16:26 +0100 Subject: [PATCH 06/11] feat: add callback transaction updates --- packages/transaction-controller/CHANGELOG.md | 2 +- ...ansactionController-method-action-types.ts | 13 ++++ .../src/TransactionController.test.ts | 67 +++++++++++++++++++ .../src/TransactionController.ts | 15 +++++ packages/transaction-controller/src/index.ts | 1 + 5 files changed, 97 insertions(+), 1 deletion(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index cafbdf43965..8ccacdc5453 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -31,7 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Web3Provider` schedules its JSON-RPC dispatch with `setTimeout`, which never fires on React Native iOS when the timer pump is starved, blocking `addTransaction` indefinitely and preventing dapp confirmations from appearing ([MetaMask/metamask-mobile#32863](https://github.com/MetaMask/metamask-mobile/issues/32863)) ### Added -- Add `beginAtomicBatchUpdate` for coherent multi-call updates with monotonic revisions and revision-bound gas preparation, and export `updateEIP7702BatchData` for synchronous indexed updates to nested transaction calldata ([#9543](https://github.com/MetaMask/core/pull/9543)) +- Add `beginAtomicBatchUpdate` for coherent multi-call updates with monotonic revisions and revision-bound gas preparation, add `updateTransactionCallback` for atomic callback-based metadata updates, and export `updateEIP7702BatchData` for synchronous indexed updates to nested transaction calldata ([#9543](https://github.com/MetaMask/core/pull/9543)) ## [69.0.0] diff --git a/packages/transaction-controller/src/TransactionController-method-action-types.ts b/packages/transaction-controller/src/TransactionController-method-action-types.ts index 8cd278cc90f..6b83749e818 100644 --- a/packages/transaction-controller/src/TransactionController-method-action-types.ts +++ b/packages/transaction-controller/src/TransactionController-method-action-types.ts @@ -134,6 +134,18 @@ export type TransactionControllerUpdateTransactionAction = { handler: TransactionController['updateTransaction']; }; +/** + * Updates an existing transaction using a callback. + * + * @param transactionId - ID of the transaction to update. + * @param callback - Function that updates the transaction metadata. + * @returns The updated transaction metadata. + */ +export type TransactionControllerUpdateTransactionCallbackAction = { + type: `TransactionController:updateTransactionCallback`; + handler: TransactionController['updateTransactionCallback']; +}; + /** * Mark a transaction as failed, transitioning it through the standard failure * path. @@ -453,6 +465,7 @@ export type TransactionControllerMethodActions = | TransactionControllerEstimateGasBatchAction | TransactionControllerEstimateGasBufferedAction | TransactionControllerUpdateTransactionAction + | TransactionControllerUpdateTransactionCallbackAction | TransactionControllerFailTransactionAction | TransactionControllerUpdateSecurityAlertResponseAction | TransactionControllerWipeTransactionsAction diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 54aa4439b3f..a31a320635e 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -4925,6 +4925,50 @@ describe('TransactionController', () => { }); }); + describe('updateTransactionCallback', () => { + it('updates multiple properties using a callback and returns the updated transaction', () => { + const { controller } = setupController({ + options: { + state: { + transactions: [TRANSACTION_META_MOCK], + }, + }, + }); + + const result = controller.updateTransactionCallback( + TRANSACTION_META_MOCK.id, + (transactionMeta) => { + transactionMeta.requiredAssets = [ + { + address: ACCOUNT_2_MOCK, + amount: '0x1', + standard: 'erc20', + }, + ]; + transactionMeta.txParams.value = '0x2'; + }, + ); + + expect(result).toStrictEqual(controller.state.transactions[0]); + expect(result.requiredAssets).toStrictEqual([ + { + address: ACCOUNT_2_MOCK, + amount: '0x1', + standard: 'erc20', + }, + ]); + expect(result.txParams.value).toBe('0x2'); + }); + + it('throws if the transaction does not exist', () => { + const { controller } = setupController(); + + expect(() => + controller.updateTransactionCallback('missing-id', () => undefined), + ).toThrow('Cannot update transaction as ID not found - missing-id'); + }); + }); + describe('updateTransactionGasFees', () => { it('throws if transaction does not exist', async () => { const { controller } = setupController(); @@ -8637,6 +8681,29 @@ describe('TransactionController', () => { }); }); + describe('TransactionController:updateTransactionCallback', () => { + it('calls updateTransactionCallback via messenger', () => { + const { controller, messenger } = setupController({ + options: { + state: { + transactions: [TRANSACTION_META_MOCK], + }, + }, + }); + + const result = messenger.call( + 'TransactionController:updateTransactionCallback', + TRANSACTION_META_MOCK.id, + (transactionMeta) => { + transactionMeta.txParams.value = '0x1'; + }, + ); + + expect(result).toStrictEqual(controller.state.transactions[0]); + expect(result.txParams.value).toBe('0x1'); + }); + }); + describe('TransactionController:getGasFeeTokens', () => { it('returns gas fee tokens', async () => { const { messenger } = setupController(); diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 749836c1b61..d14ed1e2945 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -705,6 +705,7 @@ const MESSENGER_EXPOSED_METHODS = [ 'updateSecurityAlertResponse', 'updateSelectedGasFeeToken', 'updateTransaction', + 'updateTransactionCallback', 'updateTransactionGasFees', 'wipeTransactions', ] as const; @@ -1626,6 +1627,20 @@ export class TransactionController extends BaseController< log('Transaction updated', { transactionId, note }); } + /** + * Updates an existing transaction using a callback. + * + * @param transactionId - ID of the transaction to update. + * @param callback - Function that updates the transaction metadata. + * @returns The updated transaction metadata. + */ + updateTransactionCallback( + transactionId: string, + callback: (transactionMeta: TransactionMeta) => TransactionMeta | void, + ): Readonly { + return this.#updateTransactionInternal({ transactionId }, callback); + } + /** * Mark a transaction as failed, transitioning it through the standard failure * path. diff --git a/packages/transaction-controller/src/index.ts b/packages/transaction-controller/src/index.ts index ffd3efdff6b..18bffc1f65f 100644 --- a/packages/transaction-controller/src/index.ts +++ b/packages/transaction-controller/src/index.ts @@ -36,6 +36,7 @@ export type { TransactionControllerGetTransactionsAction, TransactionControllerUpdateCustodialTransactionAction, TransactionControllerUpdateTransactionAction, + TransactionControllerUpdateTransactionCallbackAction, TransactionControllerHandleMethodDataAction, TransactionControllerIsAtomicBatchSupportedAction, TransactionControllerStopTransactionAction, From 9a9088b794298ab49bf1075643645a21da5195de Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Wed, 22 Jul 2026 12:50:26 +0100 Subject: [PATCH 07/11] refactor: make transaction pay amount updates synchronous --- packages/transaction-controller/CHANGELOG.md | 2 +- ...ansactionController-method-action-types.ts | 13 - .../src/TransactionController.test.ts | 263 ------------------ .../src/TransactionController.ts | 186 +++---------- packages/transaction-controller/src/index.ts | 4 - packages/transaction-controller/src/types.ts | 41 --- .../transaction-pay-controller/CHANGELOG.md | 2 +- ...actionPayController-method-action-types.ts | 2 +- .../src/TransactionPayController.test.ts | 145 +++++++--- .../src/TransactionPayController.ts | 43 +-- .../src/strategy/relay/relay-quotes.test.ts | 60 ---- .../src/strategy/relay/relay-quotes.ts | 21 +- .../src/tests/messenger-mock.ts | 12 +- .../transaction-pay-controller/src/types.ts | 10 +- .../src/utils/quotes.test.ts | 154 +--------- .../src/utils/quotes.ts | 57 ++-- 16 files changed, 199 insertions(+), 816 deletions(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 8ccacdc5453..de71f1e99a5 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -31,7 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Web3Provider` schedules its JSON-RPC dispatch with `setTimeout`, which never fires on React Native iOS when the timer pump is starved, blocking `addTransaction` indefinitely and preventing dapp confirmations from appearing ([MetaMask/metamask-mobile#32863](https://github.com/MetaMask/metamask-mobile/issues/32863)) ### Added -- Add `beginAtomicBatchUpdate` for coherent multi-call updates with monotonic revisions and revision-bound gas preparation, add `updateTransactionCallback` for atomic callback-based metadata updates, and export `updateEIP7702BatchData` for synchronous indexed updates to nested transaction calldata ([#9543](https://github.com/MetaMask/core/pull/9543)) +- Add `updateTransactionCallback` for atomic callback-based metadata updates and export `updateEIP7702BatchData` for synchronous indexed updates to nested transaction calldata ([#9543](https://github.com/MetaMask/core/pull/9543)) ## [69.0.0] diff --git a/packages/transaction-controller/src/TransactionController-method-action-types.ts b/packages/transaction-controller/src/TransactionController-method-action-types.ts index 6b83749e818..787ce1a4995 100644 --- a/packages/transaction-controller/src/TransactionController-method-action-types.ts +++ b/packages/transaction-controller/src/TransactionController-method-action-types.ts @@ -370,18 +370,6 @@ export type TransactionControllerAbortTransactionSigningAction = { handler: TransactionController['abortTransactionSigning']; }; -/** - * Atomically updates all amount-dependent data in an atomic batch and starts - * revision-bound local preparation. - * - * @param request - Complete atomic batch update. - * @returns The synchronously published revision and its preparation handle. - */ -export type TransactionControllerBeginAtomicBatchUpdateAction = { - type: `TransactionController:beginAtomicBatchUpdate`; - handler: TransactionController['beginAtomicBatchUpdate']; -}; - /** * Update the transaction data of a single nested transaction within an atomic batch transaction. * @@ -482,7 +470,6 @@ export type TransactionControllerMethodActions = | TransactionControllerGetLayer1GasFeeAction | TransactionControllerClearUnapprovedTransactionsAction | TransactionControllerAbortTransactionSigningAction - | TransactionControllerBeginAtomicBatchUpdateAction | TransactionControllerUpdateAtomicBatchDataAction | TransactionControllerUpdateSelectedGasFeeTokenAction | TransactionControllerUpdateRequiredTransactionIdsAction diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index a31a320635e..df6b0267a1d 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -7662,269 +7662,6 @@ describe('TransactionController', () => { }); }); - describe('beginAtomicBatchUpdate', () => { - const requiredAssets = [ - { - address: ACCOUNT_2_MOCK, - amount: '0x64' as Hex, - standard: 'erc20', - }, - ]; - - function setupAtomicBatchController( - transactionOverrides: Partial = {}, - ): ReturnType { - return setupController({ - options: { - state: { - transactions: [ - { - ...TRANSACTION_META_MOCK, - status: TransactionStatus.unapproved, - nestedTransactions: [ - { to: ACCOUNT_2_MOCK, data: '0x1234' }, - { to: ACCOUNT_2_MOCK, data: '0x4567' }, - ], - ...transactionOverrides, - }, - ], - }, - }, - updateToInitialState: true, - }); - } - - it('publishes all nested updates and required assets synchronously in one revision', async () => { - const { controller } = setupAtomicBatchController(); - - const result = controller.beginAtomicBatchUpdate({ - transactionId: TRANSACTION_META_MOCK.id, - requiredAssets, - nestedTransactionUpdates: [ - { transactionIndex: 0, transactionData: '0xAAAA' }, - { transactionIndex: 1, transactionData: '0xBBBB' }, - ], - }); - - expect(result).toStrictEqual({ - revision: 1, - transaction: expect.objectContaining({ - requiredAssets, - transactionRevision: 1, - }), - preparation: expect.any(Promise), - }); - expect( - controller.state.transactions[0].nestedTransactions?.map( - ({ data }) => data, - ), - ).toStrictEqual(['0xAAAA', '0xBBBB']); - expect(controller.state.transactions[0].txParams.data).toContain('aaaa'); - expect(controller.state.transactions[0].txParams.data).toContain('bbbb'); - - expect(await result.preparation).toMatchObject({ - revision: 1, - status: 'prepared', - }); - expect(updateGasMock).toHaveBeenCalledTimes(1); - }); - - it.each([ - { - expectedError: 'at least one nested transaction update', - name: 'an empty patch', - updates: [], - }, - { - expectedError: 'Duplicate nested transaction index - 0', - name: 'duplicate indexes', - updates: [ - { transactionIndex: 0, transactionData: '0xAAAA' as Hex }, - { transactionIndex: 0, transactionData: '0xBBBB' as Hex }, - ], - }, - { - expectedError: 'Nested transaction not found with index - 2', - name: 'an invalid index', - updates: [{ transactionIndex: 2, transactionData: '0xAAAA' as Hex }], - }, - ])( - 'rejects $name without publishing state', - ({ expectedError, updates }) => { - const { controller } = setupAtomicBatchController(); - const stateBeforeUpdate = controller.state; - - expect(() => - controller.beginAtomicBatchUpdate({ - transactionId: TRANSACTION_META_MOCK.id, - requiredAssets, - nestedTransactionUpdates: updates, - }), - ).toThrow(expectedError); - expect(controller.state).toBe(stateBeforeUpdate); - expect(updateGasMock).not.toHaveBeenCalled(); - }, - ); - - it('clears stale revision-bound preparation metadata while preserving non-gas reverts', async () => { - const gasPreparation = createDeferredPromise(); - const simulationRevert = { message: 'Simulation reverted' }; - const receiptRevert = { message: 'Receipt reverted' }; - updateGasMock.mockImplementationOnce(async () => { - await gasPreparation.promise; - }); - const { controller } = setupAtomicBatchController({ - gasLimitNoBuffer: '0x111', - gasUsed: '0x222', - revert: { - gas: { message: 'Gas reverted' }, - receipt: receiptRevert, - simulation: simulationRevert, - }, - securityAlertResponse: { - reason: 'Previous revision warning', - result_type: 'Warning', - }, - simulationData: SIMULATION_DATA_RESULT_MOCK, - simulationFails: { - debug: {}, - reason: 'Previous gas estimate failed', - }, - txParams: { - ...TRANSACTION_META_MOCK.txParams, - gas: '0x333', - }, - }); - - const result = controller.beginAtomicBatchUpdate({ - transactionId: TRANSACTION_META_MOCK.id, - requiredAssets, - nestedTransactionUpdates: [ - { transactionIndex: 0, transactionData: '0xAAAA' }, - ], - }); - const transaction = controller.state.transactions[0]; - - expect(transaction.txParams.gas).toBeUndefined(); - expect(transaction.gasLimitNoBuffer).toBeUndefined(); - expect(transaction.gasUsed).toBeUndefined(); - expect(transaction.simulationData).toBeUndefined(); - expect(transaction.simulationFails).toBeUndefined(); - expect(transaction.securityAlertResponse).toBeUndefined(); - expect(transaction.revert).toStrictEqual({ - receipt: receiptRevert, - simulation: simulationRevert, - }); - - gasPreparation.resolve(); - await result.preparation; - }); - - it('commits successful gas preparation before propagating a simulation failure', async () => { - const simulationError = new Error('Simulation failed'); - updateGasMock.mockImplementationOnce(async ({ txMeta }) => { - txMeta.txParams.gas = '0x123'; - txMeta.gasLimitNoBuffer = '0x100'; - }); - getGasFeeTokensMock.mockRejectedValueOnce(simulationError); - const { controller } = setupAtomicBatchController(); - - const { preparation } = controller.beginAtomicBatchUpdate({ - transactionId: TRANSACTION_META_MOCK.id, - requiredAssets, - nestedTransactionUpdates: [ - { transactionIndex: 0, transactionData: '0xAAAA' }, - ], - }); - - await expect(preparation).rejects.toThrow(simulationError); - expect(controller.state.transactions[0].txParams.gas).toBe('0x123'); - expect(controller.state.transactions[0].gasLimitNoBuffer).toBe('0x100'); - }); - - it('ignores simulation results started before the current revision', async () => { - const staleSimulation = createDeferredPromise<{ - simulationData: SimulationData; - }>(); - const { controller } = setupAtomicBatchController(); - getBalanceChangesMock.mockReturnValueOnce(staleSimulation.promise); - shouldResimulateMock.mockReturnValueOnce({ - blockTime: 123, - resimulate: true, - }); - - await controller.updateEditableParams(TRANSACTION_META_MOCK.id, {}); - expect(getBalanceChangesMock).toHaveBeenCalledTimes(1); - - const update = controller.beginAtomicBatchUpdate({ - transactionId: TRANSACTION_META_MOCK.id, - requiredAssets, - nestedTransactionUpdates: [ - { transactionIndex: 0, transactionData: '0xAAAA' }, - ], - }); - await update.preparation; - - staleSimulation.resolve({ - simulationData: { - ...SIMULATION_DATA_RESULT_MOCK, - nativeBalanceChange: undefined, - }, - }); - await flushPromises(); - - expect(controller.state.transactions[0].simulationData).toStrictEqual( - SIMULATION_DATA_RESULT_MOCK, - ); - expect(controller.state.transactions[0].transactionRevision).toBe(1); - }); - - it('increments revisions and prevents stale gas from overwriting a newer revision', async () => { - const firstGas = createDeferredPromise(); - const { controller } = setupAtomicBatchController(); - - updateGasMock - .mockImplementationOnce(async ({ txMeta }) => { - await firstGas.promise; - txMeta.txParams.gas = '0x111'; - }) - .mockImplementationOnce(async ({ txMeta }) => { - txMeta.txParams.gas = '0x222'; - }); - - const first = controller.beginAtomicBatchUpdate({ - transactionId: TRANSACTION_META_MOCK.id, - requiredAssets, - nestedTransactionUpdates: [ - { transactionIndex: 0, transactionData: '0xAAAA' }, - ], - }); - const second = controller.beginAtomicBatchUpdate({ - transactionId: TRANSACTION_META_MOCK.id, - requiredAssets, - nestedTransactionUpdates: [ - { transactionIndex: 0, transactionData: '0xBBBB' }, - ], - }); - - expect(await second.preparation).toMatchObject({ - revision: 2, - status: 'prepared', - }); - firstGas.resolve(); - expect(await first.preparation).toMatchObject({ - revision: 1, - status: 'superseded', - }); - - expect(controller.state.transactions[0].transactionRevision).toBe(2); - expect(controller.state.transactions[0].txParams.gas).toBe('0x222'); - expect( - controller.state.transactions[0].nestedTransactions?.[0].data, - ).toBe('0xBBBB'); - }); - }); - describe('updateAtomicBatchData', () => { /** * Template for updateAtomicBatchData test. diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index d14ed1e2945..2f026a4d3fe 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -678,7 +678,6 @@ const MESSENGER_EXPOSED_METHODS = [ 'addTransaction', 'addTransactionBatch', 'approveTransactionsWithSameNonce', - 'beginAtomicBatchUpdate', 'clearUnapprovedTransactions', 'confirmExternalTransaction', 'emulateNewTransaction', @@ -2575,85 +2574,6 @@ export class TransactionController extends BaseController< this.#signAbortCallbacks.delete(transactionId); } - /** - * Atomically updates all amount-dependent data in an atomic batch and starts - * revision-bound local preparation. - * - * @param request - Complete atomic batch update. - * @returns The synchronously published revision and its preparation handle. - */ - beginAtomicBatchUpdate( - request: BeginAtomicBatchUpdateRequest, - ): BeginAtomicBatchUpdateResult { - const { transactionId, requiredAssets, nestedTransactionUpdates } = request; - const currentTransaction = this.#getTransaction(transactionId); - let revision = 0; - - if (!currentTransaction) { - throw new Error( - `Cannot update transaction as ID not found - ${transactionId}`, - ); - } - - if (nestedTransactionUpdates.length === 0) { - throw new Error( - 'Atomic batch update requires at least one nested transaction update', - ); - } - - const { nestedTransactions, transactionData } = updateEIP7702BatchData( - currentTransaction.txParams.from as Hex, - currentTransaction.nestedTransactions ?? [], - nestedTransactionUpdates, - ); - - log('Beginning atomic batch update', request); - - const updatedTransactionMeta = this.#updateTransactionInternal( - { transactionId, skipResimulateCheck: true }, - (transactionMeta) => { - revision = (transactionMeta.transactionRevision ?? 0) + 1; - transactionMeta.nestedTransactions = nestedTransactions; - transactionMeta.requiredAssets = requiredAssets; - transactionMeta.transactionRevision = revision; - transactionMeta.txParams.data = transactionData; - transactionMeta.txParams.gas = undefined; - transactionMeta.gasLimitNoBuffer = undefined; - transactionMeta.gasUsed = undefined; - transactionMeta.securityAlertResponse = undefined; - transactionMeta.simulationData = undefined; - transactionMeta.simulationFails = undefined; - - if (transactionMeta.revert) { - delete transactionMeta.revert.gas; - - if ( - !transactionMeta.revert.simulation && - !transactionMeta.revert.receipt - ) { - transactionMeta.revert = undefined; - } - } - }, - ); - - const transaction = cloneDeep(updatedTransactionMeta); - const draftTransaction = cloneDeep({ - ...transaction, - txParams: { - ...transaction.txParams, - // Clear existing gas to force estimation. - gas: undefined, - }, - }); - - return { - revision, - transaction, - preparation: this.#prepareAtomicBatchUpdate(draftTransaction, revision), - }; - } - /** * Update the transaction data of a single nested transaction within an atomic batch transaction. * @@ -2686,72 +2606,45 @@ export class TransactionController extends BaseController< ); } - const { preparation, transaction } = this.beginAtomicBatchUpdate({ - transactionId, - requiredAssets: currentTransaction.requiredAssets ?? [], - nestedTransactionUpdates: [{ transactionIndex, transactionData }], - }); - - await preparation; - - return transaction.txParams.data as Hex; - } - - async #prepareAtomicBatchUpdate( - draftTransaction: TransactionMeta, - revision: number, - ): Promise { - const [gasPreparationResult, simulationPreparationResult] = - await Promise.allSettled([ - this.#updateGasEstimate(draftTransaction), - this.#isSimulationEnabled() - ? this.#updateSimulationData(draftTransaction) - : Promise.resolve(), - ]); - - if (gasPreparationResult.status === 'rejected') { - throw gasPreparationResult.reason; - } - - const currentTransaction = this.#getTransaction(draftTransaction.id); - - if (currentTransaction?.transactionRevision !== revision) { - return { - revision, - status: 'superseded', - transaction: cloneDeep(currentTransaction ?? draftTransaction), - }; - } - - const preparedTransaction = this.#updateTransactionInternal( - { - transactionId: draftTransaction.id, - skipResimulateCheck: true, - }, + const { nestedTransactions, transactionData: updatedTransactionData } = + updateEIP7702BatchData( + currentTransaction.txParams.from as Hex, + currentTransaction.nestedTransactions ?? [], + [{ transactionIndex, transactionData }], + ); + const updatedTransactionMeta = this.#updateTransactionInternal( + { transactionId }, (transactionMeta) => { - transactionMeta.txParams.gas = draftTransaction.txParams.gas; - transactionMeta.simulationFails = draftTransaction.simulationFails; - transactionMeta.gasLimitNoBuffer = draftTransaction.gasLimitNoBuffer; - - const draftGasRevert = draftTransaction.revert?.gas; - if (draftGasRevert) { - transactionMeta.revert = { - ...transactionMeta.revert, - gas: draftGasRevert, - }; - } + transactionMeta.nestedTransactions = nestedTransactions; + transactionMeta.txParams.data = updatedTransactionData; }, ); + const draftTransaction = cloneDeep({ + ...updatedTransactionMeta, + txParams: { + ...updatedTransactionMeta.txParams, + // Clear existing gas to force estimation. + gas: undefined, + }, + }); - if (simulationPreparationResult.status === 'rejected') { - throw simulationPreparationResult.reason; - } + await this.#updateGasEstimate(draftTransaction); - return { - revision, - status: 'prepared', - transaction: cloneDeep(preparedTransaction), - }; + this.#updateTransactionInternal({ transactionId }, (transactionMeta) => { + transactionMeta.txParams.gas = draftTransaction.txParams.gas; + transactionMeta.simulationFails = draftTransaction.simulationFails; + transactionMeta.gasLimitNoBuffer = draftTransaction.gasLimitNoBuffer; + + const draftGasRevert = draftTransaction.revert?.gas; + if (draftGasRevert) { + transactionMeta.revert = { + ...transactionMeta.revert, + gas: draftGasRevert, + }; + } + }); + + return updatedTransactionData; } /** @@ -4240,17 +4133,6 @@ export class TransactionController extends BaseController< return; } - if ( - latestTransactionMeta.transactionRevision !== - transactionMeta.transactionRevision - ) { - log('Ignoring stale simulation data', { - transactionId, - revision: transactionMeta.transactionRevision, - }); - return; - } - const updatedTransactionMeta = this.#updateTransactionInternal( { transactionId, diff --git a/packages/transaction-controller/src/index.ts b/packages/transaction-controller/src/index.ts index 18bffc1f65f..d341b2298bc 100644 --- a/packages/transaction-controller/src/index.ts +++ b/packages/transaction-controller/src/index.ts @@ -50,7 +50,6 @@ export type { TransactionControllerGetLayer1GasFeeAction, TransactionControllerClearUnapprovedTransactionsAction, TransactionControllerAbortTransactionSigningAction, - TransactionControllerBeginAtomicBatchUpdateAction, TransactionControllerUpdateAtomicBatchDataAction, TransactionControllerWipeTransactionsAction, TransactionControllerUpdateSecurityAlertResponseAction, @@ -68,10 +67,7 @@ export type { AddTransactionOptions, AfterAddHook, Authorization, - AtomicBatchPreparationResult, AuthorizationList, - BeginAtomicBatchUpdateRequest, - BeginAtomicBatchUpdateResult, BatchTransaction, BatchTransactionParams, BeforeSignHook, diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index 4aabe98538c..5b5315297da 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -265,11 +265,6 @@ export type TransactionMeta = { */ id: string; - /** - * Monotonic revision assigned whenever the atomic batch calldata is updated. - */ - transactionRevision?: number; - /** * Whether the transaction is signed externally. * No signing will be performed in the client and the `nonce` will be `undefined`. @@ -2345,42 +2340,6 @@ export type NestedTransactionUpdate = { transactionData: Hex; }; -/** Request to atomically update all amount-dependent batch data. */ -export type BeginAtomicBatchUpdateRequest = { - /** ID of the atomic batch transaction. */ - transactionId: string; - - /** Complete assets required by the updated transaction. */ - requiredAssets: RequiredAsset[]; - - /** Complete set of nested transaction calldata updates. */ - nestedTransactionUpdates: NestedTransactionUpdate[]; -}; - -/** Result of revision-bound local preparation. */ -export type AtomicBatchPreparationResult = { - /** Revision for which preparation ran. */ - revision: number; - - /** Whether the prepared metadata was committed or superseded. */ - status: 'prepared' | 'superseded'; - - /** Prepared transaction, or the current transaction if superseded. */ - transaction: TransactionMeta; -}; - -/** Synchronous result returned when an atomic update begins. */ -export type BeginAtomicBatchUpdateResult = { - /** Monotonic transaction revision assigned to the update. */ - revision: number; - - /** Coherent transaction snapshot published for this revision. */ - transaction: TransactionMeta; - - /** Revision-bound local gas preparation. */ - preparation: Promise; -}; - /** * Decoded revert from a single lifecycle source. */ diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index 52037dbdfc4..03a698fdee7 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -51,7 +51,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/assets-controller` from `^11.0.0` to `^11.1.0` ([#9579](https://github.com/MetaMask/core/pull/9579)) ### Added -- Add explicit `updateAmount` orchestration with complete-patch validation, in-flight intent deduplication, and revision-bound Relay quote publication ([#9543](https://github.com/MetaMask/core/pull/9543)) +- Add explicit `updateAmount` orchestration with complete-patch validation, in-flight intent deduplication, and synchronous coherent transaction updates before quote generation ([#9543](https://github.com/MetaMask/core/pull/9543)) ## [25.0.0] diff --git a/packages/transaction-pay-controller/src/TransactionPayController-method-action-types.ts b/packages/transaction-pay-controller/src/TransactionPayController-method-action-types.ts index c658439b938..652c37fc42b 100644 --- a/packages/transaction-pay-controller/src/TransactionPayController-method-action-types.ts +++ b/packages/transaction-pay-controller/src/TransactionPayController-method-action-types.ts @@ -21,7 +21,7 @@ export type TransactionPayControllerSetTransactionConfigAction = { /** * Prepares and atomically commits an exact transaction amount, then launches - * one quote generation joined to revision-bound local preparation. + * one quote generation for the updated transaction. * Identical in-flight intents share the same promise; different intents * supersede and abort earlier work. * diff --git a/packages/transaction-pay-controller/src/TransactionPayController.test.ts b/packages/transaction-pay-controller/src/TransactionPayController.test.ts index 9c31feb7075..05f1d035959 100644 --- a/packages/transaction-pay-controller/src/TransactionPayController.test.ts +++ b/packages/transaction-pay-controller/src/TransactionPayController.test.ts @@ -1,9 +1,6 @@ /* eslint-disable no-new */ -import type { - BeginAtomicBatchUpdateResult, - TransactionMeta, -} from '@metamask/transaction-controller'; +import type { TransactionMeta } from '@metamask/transaction-controller'; import type { Hex, Json } from '@metamask/utils'; import { createDeferredPromise } from '@metamask/utils'; @@ -23,7 +20,7 @@ import type { UpdateTransactionDataCallback, } from './types.js'; import { getStrategyOrder } from './utils/feature-flags.js'; -import { updateQuotes } from './utils/quotes.js'; +import { abortQuotes, updateQuotes } from './utils/quotes.js'; import { updateSourceAmounts } from './utils/source-amounts.js'; import { getTransaction, @@ -51,6 +48,7 @@ describe('TransactionPayController', () => { ); const getTransactionMock = jest.mocked(getTransaction); const updateSourceAmountsMock = jest.mocked(updateSourceAmounts); + const abortQuotesMock = jest.mocked(abortQuotes); const updateQuotesMock = jest.mocked(updateQuotes); const subscribeTransactionChangesMock = jest.mocked( subscribeTransactionChanges, @@ -58,7 +56,7 @@ describe('TransactionPayController', () => { const subscribeAssetChangesMock = jest.mocked(subscribeAssetChanges); const getStrategyOrderMock = jest.mocked(getStrategyOrder); let messenger: TransactionPayControllerMessenger; - let beginAtomicBatchUpdateMock: jest.Mock; + let updateTransactionCallbackMock: jest.Mock; let getKeyringControllerStateMock: jest.Mock; /** @@ -82,7 +80,7 @@ describe('TransactionPayController', () => { const mocks = getMessengerMock({ skipRegister: true }); messenger = mocks.messenger; - beginAtomicBatchUpdateMock = mocks.beginAtomicBatchUpdateMock; + updateTransactionCallbackMock = mocks.updateTransactionCallbackMock; getKeyringControllerStateMock = mocks.getKeyringControllerStateMock; getKeyringControllerStateMock.mockReturnValue({ @@ -136,18 +134,21 @@ describe('TransactionPayController', () => { { transactionIndex: 1, transactionData: '0xBBBB' as Hex }, ]; - function mockAtomicUpdate(): BeginAtomicBatchUpdateResult { - const result = { - revision: 1, - transaction: { ...transaction, transactionRevision: 1 }, - preparation: Promise.resolve({ - revision: 1, - status: 'prepared' as const, - transaction: { ...transaction, transactionRevision: 1 }, - }), + function mockTransactionUpdateCallback(): TransactionMeta { + const updatedTransaction = { + ...transaction, + nestedTransactions: transaction.nestedTransactions?.map( + (nestedTransaction) => ({ ...nestedTransaction }), + ), + txParams: { ...transaction.txParams }, }; - beginAtomicBatchUpdateMock.mockReturnValue(result); - return result; + updateTransactionCallbackMock.mockImplementation( + (_transactionId, callback) => { + callback(updatedTransaction); + return updatedTransaction; + }, + ); + return updatedTransaction; } function getStateWithOldQuote(): TransactionPayControllerOptions['state'] { @@ -223,7 +224,7 @@ describe('TransactionPayController', () => { amountHuman: '1.23', }), ).rejects.toThrow('Transaction amount preparation is not applicable'); - expect(beginAtomicBatchUpdateMock).not.toHaveBeenCalled(); + expect(updateTransactionCallbackMock).not.toHaveBeenCalled(); }); it('passes the exact human amount and commits the complete patch once', async () => { @@ -235,7 +236,7 @@ describe('TransactionPayController', () => { requiredNestedTransactionIndexes: [0, 1], }); getTransactionMock.mockReturnValue(transaction); - mockAtomicUpdate(); + const updatedTransaction = mockTransactionUpdateCallback(); const controller = createController({ prepareTransactionAmount }); controller.setTransactionConfig(TRANSACTION_ID_MOCK, () => undefined); updateQuotesMock.mockClear(); @@ -252,17 +253,26 @@ describe('TransactionPayController', () => { signal: expect.any(AbortSignal), transaction, }); - expect(beginAtomicBatchUpdateMock).toHaveBeenCalledWith({ - transactionId: TRANSACTION_ID_MOCK, - requiredAssets, - nestedTransactionUpdates, - }); + expect(abortQuotesMock).toHaveBeenCalledWith(TRANSACTION_ID_MOCK); + expect(abortQuotesMock.mock.invocationCallOrder[0]).toBeLessThan( + prepareTransactionAmount.mock.invocationCallOrder[0], + ); + expect(updateTransactionCallbackMock).toHaveBeenCalledWith( + TRANSACTION_ID_MOCK, + expect.any(Function), + ); + expect(updatedTransaction.requiredAssets).toStrictEqual(requiredAssets); + expect( + updatedTransaction.nestedTransactions?.map(({ data }) => data), + ).toStrictEqual(['0xAAAA', '0xBBBB']); + expect(updatedTransaction.txParams.data).toContain('aaaa'); + expect(updatedTransaction.txParams.data).toContain('bbbb'); expect(updateQuotesMock).toHaveBeenCalledTimes(1); - expect(updateQuotesMock).toHaveBeenCalledWith( - expect.objectContaining({ - transactionRevision: 1, - transactionPreparation: expect.any(Promise), - }), + expect(updateQuotesMock.mock.calls[0][0]).not.toHaveProperty( + 'transactionPreparation', + ); + expect(updateQuotesMock.mock.calls[0][0]).not.toHaveProperty( + 'transactionRevision', ); }); @@ -282,11 +292,11 @@ describe('TransactionPayController', () => { expectOldQuoteInvalidated(controller, true); await expect(result).rejects.toThrow(callbackError); expectOldQuoteInvalidated(controller, false); - expect(beginAtomicBatchUpdateMock).not.toHaveBeenCalled(); + expect(updateTransactionCallbackMock).not.toHaveBeenCalled(); expect(updateQuotesMock).not.toHaveBeenCalled(); }); - it('keeps an old quote cleared when revision preparation or vendor quoting fails', async () => { + it('keeps an old quote cleared when vendor quoting fails', async () => { const pipelineError = new Error('Quote pipeline failed'); const controller = createController({ prepareTransactionAmount: jest.fn().mockResolvedValue({ @@ -299,7 +309,7 @@ describe('TransactionPayController', () => { state: getStateWithOldQuote(), }); getTransactionMock.mockReturnValue(transaction); - mockAtomicUpdate(); + mockTransactionUpdateCallback(); updateQuotesMock.mockRejectedValue(pipelineError); const result = controller.updateAmount({ @@ -326,7 +336,7 @@ describe('TransactionPayController', () => { state: getStateWithOldQuote(), }); getTransactionMock.mockReturnValue(transaction); - mockAtomicUpdate(); + mockTransactionUpdateCallback(); const first = controller.updateAmount({ transactionId: TRANSACTION_ID_MOCK, @@ -364,7 +374,7 @@ describe('TransactionPayController', () => { .fn() .mockReturnValue(preparation.promise); getTransactionMock.mockReturnValue(transaction); - mockAtomicUpdate(); + mockTransactionUpdateCallback(); const controller = createController({ prepareTransactionAmount }); controller.setTransactionConfig(TRANSACTION_ID_MOCK, () => undefined); const request = { @@ -407,7 +417,7 @@ describe('TransactionPayController', () => { requiredNestedTransactionIndexes: [0, 1], }); getTransactionMock.mockReturnValue(transaction); - mockAtomicUpdate(); + mockTransactionUpdateCallback(); const controller = createController({ prepareTransactionAmount }); controller.setTransactionConfig(TRANSACTION_ID_MOCK, () => undefined); @@ -426,7 +436,49 @@ describe('TransactionPayController', () => { expect(await second).toBe(true); }); - it('rejects a partial patch without committing a revision', async () => { + it('rejects an update if the current transaction has no nested transactions', async () => { + const prepareTransactionAmount = jest.fn().mockResolvedValue({ + kind: 'prepared', + amountRaw: '123', + requiredAssets, + nestedTransactionUpdates, + requiredNestedTransactionIndexes: [0, 1], + }); + getTransactionMock.mockReturnValue(transaction); + updateTransactionCallbackMock.mockImplementation( + (_transactionId, callback) => { + const currentTransaction = { + ...transaction, + nestedTransactions: undefined, + txParams: { ...transaction.txParams }, + }; + callback(currentTransaction); + return currentTransaction; + }, + ); + const controller = createController({ + prepareTransactionAmount, + state: { + transactionData: { + [TRANSACTION_ID_MOCK]: { + fiatPayment: {}, + isLoading: false, + tokens: [], + }, + }, + }, + }); + + await expect( + controller.updateAmount({ + transactionId: TRANSACTION_ID_MOCK, + amountHuman: '1.23', + }), + ).rejects.toThrow('Nested transaction not found with index - 0'); + expect(updateQuotesMock).not.toHaveBeenCalled(); + }); + + it('rejects a partial patch without committing the transaction', async () => { const controller = createController({ prepareTransactionAmount: jest.fn().mockResolvedValue({ kind: 'prepared', @@ -444,7 +496,7 @@ describe('TransactionPayController', () => { amountHuman: '1.23', }), ).rejects.toThrow('incomplete patch'); - expect(beginAtomicBatchUpdateMock).not.toHaveBeenCalled(); + expect(updateTransactionCallbackMock).not.toHaveBeenCalled(); }); it('suppresses the listener quote launch caused by its atomic publication', async () => { @@ -461,13 +513,16 @@ describe('TransactionPayController', () => { updateQuotesMock.mockClear(); const listenerUpdateTransactionData = subscribeTransactionChangesMock.mock.calls[0][1]; - const atomicUpdate = mockAtomicUpdate(); - beginAtomicBatchUpdateMock.mockImplementationOnce((request) => { - listenerUpdateTransactionData(request.transactionId, (data) => { - data.tokens = [{ address: TOKEN_ADDRESS_MOCK }] as never; - }); - return atomicUpdate; - }); + const updatedTransaction = mockTransactionUpdateCallback(); + updateTransactionCallbackMock.mockImplementationOnce( + (transactionId, callback) => { + listenerUpdateTransactionData(transactionId, (data) => { + data.tokens = [{ address: TOKEN_ADDRESS_MOCK }] as never; + }); + callback(updatedTransaction); + return updatedTransaction; + }, + ); await controller.updateAmount({ transactionId: TRANSACTION_ID_MOCK, diff --git a/packages/transaction-pay-controller/src/TransactionPayController.ts b/packages/transaction-pay-controller/src/TransactionPayController.ts index ac2f23dace8..329f6c1eeef 100644 --- a/packages/transaction-pay-controller/src/TransactionPayController.ts +++ b/packages/transaction-pay-controller/src/TransactionPayController.ts @@ -1,9 +1,8 @@ import type { StateMetadata } from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; -import type { - BeginAtomicBatchUpdateResult, - TransactionMeta, -} from '@metamask/transaction-controller'; +import { updateEIP7702BatchData } from '@metamask/transaction-controller'; +import type { TransactionMeta } from '@metamask/transaction-controller'; +import type { Hex } from '@metamask/utils'; import type { Draft } from 'immer'; import { noop } from 'lodash'; @@ -33,7 +32,7 @@ import type { UpdatePaymentTokenRequest, } from './types.js'; import { getStrategyOrder } from './utils/feature-flags.js'; -import { updateQuotes } from './utils/quotes.js'; +import { abortQuotes, updateQuotes } from './utils/quotes.js'; import { updateSourceAmounts } from './utils/source-amounts.js'; import { getTransaction, @@ -207,7 +206,7 @@ export class TransactionPayController extends BaseController< /** * Prepares and atomically commits an exact transaction amount, then launches - * one quote generation joined to revision-bound local preparation. + * one quote generation for the updated transaction. * Identical in-flight intents share the same promise; different intents * supersede and abort earlier work. * @@ -268,6 +267,7 @@ export class TransactionPayController extends BaseController< transactionData.quotesLastUpdated = undefined; transactionData.totals = undefined; }); + abortQuotes(transactionId); const amountPreparation = await this.#prepareTransactionAmount({ amountHuman, @@ -285,10 +285,7 @@ export class TransactionPayController extends BaseController< throw new Error('Transaction amount preparation is not applicable'); } - const atomicUpdate = this.#beginAtomicBatchUpdate( - transactionId, - amountPreparation, - ); + this.#updateTransactionAmount(transactionId, amountPreparation); return await updateQuotes({ getStrategies: this.#getStrategiesWithFallback.bind(this), @@ -296,28 +293,34 @@ export class TransactionPayController extends BaseController< signal, transactionData: this.state.transactionData[transactionId], transactionId, - transactionPreparation: atomicUpdate.preparation, - transactionRevision: atomicUpdate.revision, updateTransactionData: this.#updateTransactionData.bind(this), }); } - #beginAtomicBatchUpdate( + #updateTransactionAmount( transactionId: string, amountPreparation: Extract< PrepareTransactionAmountResult, { kind: 'prepared' } >, - ): BeginAtomicBatchUpdateResult { + ): void { this.#quoteSuppressedTransactionIds.add(transactionId); try { - return this.messenger.call( - 'TransactionController:beginAtomicBatchUpdate', - { - transactionId, - requiredAssets: amountPreparation.requiredAssets, - nestedTransactionUpdates: amountPreparation.nestedTransactionUpdates, + this.messenger.call( + 'TransactionController:updateTransactionCallback', + transactionId, + (transactionMeta) => { + const { nestedTransactions, transactionData } = + updateEIP7702BatchData( + transactionMeta.txParams.from as Hex, + transactionMeta.nestedTransactions ?? [], + amountPreparation.nestedTransactionUpdates, + ); + + transactionMeta.nestedTransactions = nestedTransactions; + transactionMeta.requiredAssets = amountPreparation.requiredAssets; + transactionMeta.txParams.data = transactionData; }, ); } finally { diff --git a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts index ccf1a189e2b..0bf88e00f27 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts @@ -1,12 +1,10 @@ import { toHex } from '@metamask/controller-utils'; import { TransactionType } from '@metamask/transaction-controller'; import type { - AtomicBatchPreparationResult, GasFeeToken, TransactionMeta, } from '@metamask/transaction-controller'; import type { Hex } from '@metamask/utils'; -import { createDeferredPromise } from '@metamask/utils'; import { cloneDeep } from 'lodash'; import { getDefaultRemoteFeatureFlagControllerState } from '../../../../remote-feature-flag-controller/src/remote-feature-flag-controller.js'; @@ -248,64 +246,6 @@ describe('Relay Quotes Utils', () => { }); describe('getRelayQuotes', () => { - it('fetches the raw standard quote before preparation and normalizes afterward', async () => { - successfulFetchMock.mockResolvedValue({ - ok: true, - json: async () => QUOTE_MOCK, - } as never); - const transactionPreparation = - createDeferredPromise(); - - const resultPromise = getRelayQuotes({ - accountSupports7702: true, - messenger, - requests: [QUOTE_REQUEST_MOCK], - transaction: TRANSACTION_META_MOCK, - transactionPreparation: transactionPreparation.promise, - }); - - await new Promise((resolve) => process.nextTick(resolve)); - - expect(successfulFetchMock).toHaveBeenCalledTimes(1); - expect(calculateGasCostMock).not.toHaveBeenCalled(); - - transactionPreparation.resolve({ - revision: 1, - status: 'prepared', - transaction: { - ...TRANSACTION_META_MOCK, - transactionRevision: 1, - }, - }); - - expect(await resultPromise).toHaveLength(1); - expect(calculateGasCostMock).toHaveBeenCalled(); - }); - - it('rejects a raw standard quote when transaction preparation was superseded', async () => { - successfulFetchMock.mockResolvedValue({ - ok: true, - json: async () => QUOTE_MOCK, - } as never); - - await expect( - getRelayQuotes({ - accountSupports7702: true, - messenger, - requests: [QUOTE_REQUEST_MOCK], - transaction: TRANSACTION_META_MOCK, - transactionPreparation: Promise.resolve({ - revision: 1, - status: 'superseded', - transaction: TRANSACTION_META_MOCK, - }), - }), - ).rejects.toThrow('Transaction preparation was superseded'); - - expect(successfulFetchMock).toHaveBeenCalledTimes(1); - expect(calculateGasCostMock).not.toHaveBeenCalled(); - }); - it('returns quotes from Relay', async () => { successfulFetchMock.mockResolvedValue({ ok: true, diff --git a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts index 124a8f0cef9..6afdb789009 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts @@ -365,26 +365,7 @@ async function getSingleQuote( log('Fetched relay quote', quote); - let normalizationRequest = fullRequest; - const transactionPreparation = - !request.isMaxAmount && !request.isPostQuote - ? fullRequest.transactionPreparation - : undefined; - - if (transactionPreparation) { - const preparationResult = await transactionPreparation; - - if (preparationResult.status !== 'prepared') { - throw new Error('Transaction preparation was superseded'); - } - - normalizationRequest = { - ...fullRequest, - transaction: preparationResult.transaction, - }; - } - - return await normalizeQuote(quote, request, normalizationRequest); + return await normalizeQuote(quote, request, fullRequest); } catch (error) { log('Error fetching relay quote', error); throw error; diff --git a/packages/transaction-pay-controller/src/tests/messenger-mock.ts b/packages/transaction-pay-controller/src/tests/messenger-mock.ts index f485c8a2eb6..43c955e6be6 100644 --- a/packages/transaction-pay-controller/src/tests/messenger-mock.ts +++ b/packages/transaction-pay-controller/src/tests/messenger-mock.ts @@ -17,8 +17,8 @@ import type { SentinelApiServiceSimulateTransactionsAction } from '@metamask/sen import type { TransactionControllerAddTransactionAction, TransactionControllerAddTransactionBatchAction, - TransactionControllerBeginAtomicBatchUpdateAction, TransactionControllerEstimateGasAction, + TransactionControllerUpdateTransactionCallbackAction, TransactionControllerEstimateGasBatchAction, TransactionControllerGetGasFeeTokensAction, TransactionControllerGetStateAction, @@ -71,8 +71,8 @@ export function getMessengerMock({ TransactionControllerAddTransactionBatchAction['handler'] > = jest.fn(); - const beginAtomicBatchUpdateMock: jest.MockedFn< - TransactionControllerBeginAtomicBatchUpdateAction['handler'] + const updateTransactionCallbackMock: jest.MockedFn< + TransactionControllerUpdateTransactionCallbackAction['handler'] > = jest.fn(); const findNetworkClientIdByChainIdMock: jest.MockedFn< @@ -303,8 +303,8 @@ export function getMessengerMock({ } messenger.registerActionHandler( - 'TransactionController:beginAtomicBatchUpdate', - beginAtomicBatchUpdateMock, + 'TransactionController:updateTransactionCallback', + updateTransactionCallbackMock, ); messenger.registerActionHandler( @@ -316,7 +316,6 @@ export function getMessengerMock({ return { addTransactionMock, - beginAtomicBatchUpdateMock, getAssetsControllerStateMock, addTransactionBatchMock, estimateGasMock, @@ -344,6 +343,7 @@ export function getMessengerMock({ polymarketSubmitDepositWalletBatchMock, publish, simulateTransactionsMock, + updateTransactionCallbackMock, updateTransactionMock, }; } diff --git a/packages/transaction-pay-controller/src/types.ts b/packages/transaction-pay-controller/src/types.ts index c942f1b6360..1857cef8d13 100644 --- a/packages/transaction-pay-controller/src/types.ts +++ b/packages/transaction-pay-controller/src/types.ts @@ -35,12 +35,10 @@ import type { import type { RemoteFeatureFlagControllerGetStateAction } from '@metamask/remote-feature-flag-controller'; import type { SentinelApiServiceActions } from '@metamask/sentinel-api-service'; import type { - AtomicBatchPreparationResult, AuthorizationList, NestedTransactionUpdate, RequiredAsset, TransactionControllerAddTransactionBatchAction, - TransactionControllerBeginAtomicBatchUpdateAction, TransactionControllerEstimateGasAction, TransactionControllerEstimateGasBatchAction, TransactionControllerUnapprovedTransactionAddedEvent, @@ -53,6 +51,7 @@ import type { TransactionControllerGetStateAction, TransactionControllerStateChangeEvent, TransactionControllerUpdateTransactionAction, + TransactionControllerUpdateTransactionCallbackAction, TransactionMeta, } from '@metamask/transaction-controller'; import type { Hex, Json } from '@metamask/utils'; @@ -84,12 +83,12 @@ export type AllowedActions = | SentinelApiServiceActions | TransactionControllerAddTransactionAction | TransactionControllerAddTransactionBatchAction - | TransactionControllerBeginAtomicBatchUpdateAction | TransactionControllerEstimateGasAction | TransactionControllerEstimateGasBatchAction | TransactionControllerGetGasFeeTokensAction | TransactionControllerGetStateAction - | TransactionControllerUpdateTransactionAction; + | TransactionControllerUpdateTransactionAction + | TransactionControllerUpdateTransactionCallbackAction; export type AllowedEvents = | AssetsControllerStateChangeEvent @@ -722,9 +721,6 @@ export type PayStrategyGetQuotesRequest = { /** Metadata of the original target transaction. */ transaction: TransactionMeta; - - /** Revision-bound local preparation for an explicit amount update. */ - transactionPreparation?: Promise; }; /** Request to submit quotes for a transaction. */ diff --git a/packages/transaction-pay-controller/src/utils/quotes.test.ts b/packages/transaction-pay-controller/src/utils/quotes.test.ts index faa5e20e42f..fde214de3a6 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.test.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.test.ts @@ -1,11 +1,7 @@ import { TransactionStatus } from '@metamask/transaction-controller'; -import type { - AtomicBatchPreparationResult, - TransactionMeta, -} from '@metamask/transaction-controller'; +import type { TransactionMeta } from '@metamask/transaction-controller'; import type { BatchTransaction } from '@metamask/transaction-controller'; import type { Hex, Json } from '@metamask/utils'; -import { createDeferredPromise } from '@metamask/utils'; import { cloneDeep } from 'lodash'; import { TransactionPayStrategy } from '../constants.js'; @@ -19,7 +15,7 @@ import type { TransactionPayRequiredToken, } from '../types.js'; import type { UpdateQuotesRequest } from './quotes.js'; -import { refreshQuotes, updateQuotes } from './quotes.js'; +import { abortQuotes, refreshQuotes, updateQuotes } from './quotes.js'; import { checkStrategyQuoteSupport, checkStrategySupport, @@ -204,97 +200,6 @@ describe('Quotes Utils', () => { }); describe('updateQuotes', () => { - it('does not publish an executable quote until matching preparation completes', async () => { - const transactionPreparation = - createDeferredPromise(); - const preparedTransaction = { - ...TRANSACTION_META_MOCK, - transactionRevision: 1, - }; - getTransactionMock.mockReturnValue(preparedTransaction); - - const resultPromise = run({ - transactionPreparation: transactionPreparation.promise, - transactionRevision: 1, - }); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - - expect(getQuotesMock).toHaveBeenCalled(); - expect(calculateTotalsMock).not.toHaveBeenCalled(); - - transactionPreparation.resolve({ - revision: 1, - status: 'prepared', - transaction: preparedTransaction, - }); - - expect(await resultPromise).toBe(true); - expect(calculateTotalsMock).toHaveBeenCalledWith( - expect.objectContaining({ transaction: preparedTransaction }), - ); - }); - - it('discards quotes when preparation was superseded', async () => { - getTransactionMock.mockReturnValue({ - ...TRANSACTION_META_MOCK, - transactionRevision: 1, - }); - - const result = await run({ - transactionPreparation: Promise.resolve({ - revision: 1, - status: 'superseded', - transaction: TRANSACTION_META_MOCK, - }), - transactionRevision: 1, - }); - - expect(result).toBe(false); - expect(calculateTotalsMock).not.toHaveBeenCalled(); - }); - - it('discards quotes when preparation has a different revision', async () => { - getTransactionMock.mockReturnValue({ - ...TRANSACTION_META_MOCK, - transactionRevision: 1, - }); - - const result = await run({ - transactionPreparation: Promise.resolve({ - revision: 2, - status: 'prepared', - transaction: TRANSACTION_META_MOCK, - }), - transactionRevision: 1, - }); - - expect(result).toBe(false); - expect(calculateTotalsMock).not.toHaveBeenCalled(); - }); - - it('discards quotes when the latest transaction has a different revision', async () => { - getTransactionMock.mockReturnValue({ - ...TRANSACTION_META_MOCK, - transactionRevision: 2, - }); - - const result = await run({ - transactionPreparation: Promise.resolve({ - revision: 1, - status: 'prepared', - transaction: TRANSACTION_META_MOCK, - }), - transactionRevision: 1, - }); - - expect(result).toBe(false); - expect(calculateTotalsMock).not.toHaveBeenCalled(); - }); - it('aborts immediately when the external signal is already aborted', async () => { const externalController = new AbortController(); externalController.abort(); @@ -307,50 +212,6 @@ describe('Quotes Utils', () => { expect(calculateTotalsMock).not.toHaveBeenCalled(); }); - it('discards prepared quotes if the external signal aborts while preparation is pending', async () => { - const externalController = new AbortController(); - const transactionPreparation = - createDeferredPromise(); - const preparedTransaction = { - ...TRANSACTION_META_MOCK, - transactionRevision: 1, - }; - getTransactionMock.mockReturnValue(preparedTransaction); - - const resultPromise = run({ - signal: externalController.signal, - transactionPreparation: transactionPreparation.promise, - transactionRevision: 1, - }); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - - expect(getQuotesMock).toHaveBeenCalled(); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); - - externalController.abort(); - transactionPreparation.resolve({ - revision: 1, - status: 'prepared', - transaction: preparedTransaction, - }); - - expect(await resultPromise).toBe(false); - expect(calculateTotalsMock).not.toHaveBeenCalled(); - }); - it('updates quotes in state', async () => { await run(); @@ -1253,6 +1114,17 @@ describe('Quotes Utils', () => { return promise; } + it('aborts the active call explicitly', async () => { + const balance = deferred(); + getLiveTokenBalanceMock.mockReturnValueOnce(balance); + + const resultPromise = run(); + abortQuotes(TRANSACTION_ID_MOCK); + balance.resolve('5000000'); + + expect(await resultPromise).toBe(false); + }); + it('aborts the previous call so its results are not written to state', async () => { const firstBalance = deferred(); const secondBalance = deferred(); diff --git a/packages/transaction-pay-controller/src/utils/quotes.ts b/packages/transaction-pay-controller/src/utils/quotes.ts index d707b0acc97..e2ef67fd317 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.ts @@ -1,6 +1,5 @@ import { TransactionStatus } from '@metamask/transaction-controller'; import type { - AtomicBatchPreparationResult, BatchTransaction, TransactionMeta, } from '@metamask/transaction-controller'; @@ -50,8 +49,6 @@ export type UpdateQuotesRequest = { signal?: AbortSignal; transactionData: TransactionData | undefined; transactionId: string; - transactionPreparation?: Promise; - transactionRevision?: number; updateTransactionData: UpdateTransactionDataCallback; }; @@ -75,8 +72,6 @@ export async function updateQuotes( signal: externalSignal, transactionData, transactionId, - transactionPreparation, - transactionRevision, updateTransactionData, } = request; @@ -169,7 +164,6 @@ export async function updateQuotes( messenger, fiatPayment?.selectedPaymentMethodId, signal, - transactionPreparation, ); if (signal.aborted) { @@ -177,31 +171,6 @@ export async function updateQuotes( return false; } - let preparedTransaction = transaction; - - if (transactionPreparation) { - const preparationResult = await transactionPreparation; - const latestTransaction = getTransaction(transactionId, messenger); - - if ( - preparationResult.status !== 'prepared' || - preparationResult.revision !== transactionRevision || - latestTransaction?.transactionRevision !== transactionRevision - ) { - log('Discarding quotes for stale transaction revision', { - transactionId, - transactionRevision, - }); - return false; - } - - preparedTransaction = preparationResult.transaction; - } - - if (signal.aborted) { - return false; - } - // No-op quotes mark direct routes. They have no fees or amounts and the // transaction is signed and submitted locally, so totals and transaction // sync must treat them as "no quotes". @@ -215,7 +184,7 @@ export async function updateQuotes( messenger, quotes: executableQuotes as TransactionPayQuote[], tokens, - transaction: preparedTransaction, + transaction, }); log('Calculated totals', { transactionId, totals }); @@ -390,15 +359,24 @@ export async function refreshQuotes( } } +/** + * Abort the active quote request for a transaction. + * + * @param transactionId - ID of the transaction whose quote should be aborted. + */ +export function abortQuotes(transactionId: string): void { + const request = inFlightQuoteRequests.get(transactionId); + + if (request && !request.signal.aborted) { + log('Aborting quote request', { transactionId }); + request.abort(new Error('Superseded by newer quote request')); + } +} + function abortPreviousAndCreateController( transactionId: string, ): AbortController { - const previous = inFlightQuoteRequests.get(transactionId); - - if (previous && !previous.signal.aborted) { - log('Aborting previous quote request', { transactionId }); - previous.abort(new Error('Superseded by newer quote request')); - } + abortQuotes(transactionId); const controller = new AbortController(); inFlightQuoteRequests.set(transactionId, controller); @@ -665,7 +643,6 @@ async function refreshPaymentTokenBalance({ * @param messenger - Controller messenger. * @param fiatPaymentMethod - Selected fiat payment method ID, if applicable. * @param signal - Signal that aborts when the quote request is superseded. - * @param transactionPreparation - Revision-bound local preparation. * @returns An object containing batch transactions and quotes. */ async function getQuotes( @@ -679,7 +656,6 @@ async function getQuotes( messenger: TransactionPayControllerMessenger, fiatPaymentMethod?: string, signal?: AbortSignal, - transactionPreparation?: Promise, ): Promise<{ batchTransactions: BatchTransaction[]; error?: QuoteErrorInfo; @@ -725,7 +701,6 @@ async function getQuotes( requests, signal, transaction, - transactionPreparation, }; let error: QuoteErrorInfo | undefined; From 58fa1aca0c10587d69dde767207b5cf256507e4f Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Wed, 22 Jul 2026 13:36:20 +0100 Subject: [PATCH 08/11] fix: clear stale batch preparation metadata --- packages/transaction-controller/CHANGELOG.md | 4 ++ .../src/TransactionController.test.ts | 69 +++++++++++++++++++ .../src/TransactionController.ts | 17 +++++ 3 files changed, 90 insertions(+) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index de71f1e99a5..c9770cdb57e 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -33,6 +33,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `updateTransactionCallback` for atomic callback-based metadata updates and export `updateEIP7702BatchData` for synchronous indexed updates to nested transaction calldata ([#9543](https://github.com/MetaMask/core/pull/9543)) +### Fixed + +- Clear stale gas and simulation metadata synchronously when updating EIP-7702 batch calldata ([#9543](https://github.com/MetaMask/core/pull/9543)) + ## [69.0.0] ### Changed diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index df6b0267a1d..149eb4df58d 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -7727,6 +7727,75 @@ describe('TransactionController', () => { expect(result).not.toContain('4567'); }); + it('clears stale preparation metadata before gas estimation completes', async () => { + const gasPreparation = createDeferredPromise(); + const receiptRevert = { message: 'Receipt reverted' }; + const simulationRevert = { message: 'Simulation reverted' }; + updateGasMock.mockImplementationOnce(async ({ txMeta }) => { + await gasPreparation.promise; + txMeta.txParams.gas = '0x222'; + txMeta.gasLimitNoBuffer = '0x200'; + }); + const { controller } = setupController({ + options: { + state: { + transactions: [ + { + ...TRANSACTION_META_MOCK, + gasLimitNoBuffer: '0x100', + gasUsed: '0x101', + nestedTransactions: [{ to: ACCOUNT_2_MOCK, data: '0x1234' }], + revert: { + gas: { message: 'Gas reverted' }, + receipt: receiptRevert, + simulation: simulationRevert, + }, + securityAlertResponse: { + reason: 'Previous revision warning', + result_type: 'Warning', + }, + simulationData: SIMULATION_DATA_RESULT_MOCK, + simulationFails: { + debug: {}, + reason: 'Previous gas estimate failed', + }, + txParams: { + ...TRANSACTION_META_MOCK.txParams, + gas: '0x102', + }, + }, + ], + }, + }, + }); + + const updatePromise = controller.updateAtomicBatchData({ + transactionId: TRANSACTION_META_MOCK.id, + transactionIndex: 0, + transactionData: '0x89AB', + }); + const transaction = controller.state.transactions[0]; + + expect(transaction.nestedTransactions?.[0].data).toBe('0x89AB'); + expect(transaction.txParams.data).toContain('89ab'); + expect(transaction.txParams.gas).toBeUndefined(); + expect(transaction.gasLimitNoBuffer).toBeUndefined(); + expect(transaction.gasUsed).toBeUndefined(); + expect(transaction.securityAlertResponse).toBeUndefined(); + expect(transaction.simulationData).toBeUndefined(); + expect(transaction.simulationFails).toBeUndefined(); + expect(transaction.revert).toStrictEqual({ + receipt: receiptRevert, + simulation: simulationRevert, + }); + + gasPreparation.resolve(); + await updatePromise; + + expect(controller.state.transactions[0].txParams.gas).toBe('0x222'); + expect(controller.state.transactions[0].gasLimitNoBuffer).toBe('0x200'); + }); + it('updates gas', async () => { const gasMock = '0x1234'; const gasLimitNoBufferMock = '0x123'; diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 2f026a4d3fe..7534fa1676c 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -2617,6 +2617,23 @@ export class TransactionController extends BaseController< (transactionMeta) => { transactionMeta.nestedTransactions = nestedTransactions; transactionMeta.txParams.data = updatedTransactionData; + transactionMeta.txParams.gas = undefined; + transactionMeta.gasLimitNoBuffer = undefined; + transactionMeta.gasUsed = undefined; + transactionMeta.securityAlertResponse = undefined; + transactionMeta.simulationData = undefined; + transactionMeta.simulationFails = undefined; + + if (transactionMeta.revert) { + delete transactionMeta.revert.gas; + + if ( + !transactionMeta.revert.simulation && + !transactionMeta.revert.receipt + ) { + transactionMeta.revert = undefined; + } + } }, ); const draftTransaction = cloneDeep({ From 7d13ae14942cf6f418db792a8d7cd98de41f2a41 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Fri, 24 Jul 2026 15:00:25 +0100 Subject: [PATCH 09/11] docs: fix changelogs after rebase --- packages/transaction-controller/CHANGELOG.md | 12 +++++++++--- packages/transaction-pay-controller/CHANGELOG.md | 11 ++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index c9770cdb57e..98d7854c5fc 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `updateTransactionCallback` for applying coherent transaction metadata updates through the messenger ([#9543](https://github.com/MetaMask/core/pull/9543)) +- Export `updateEIP7702BatchData` for updating nested EIP-7702 batch calldata without mutating the input ([#9543](https://github.com/MetaMask/core/pull/9543)) + ### Changed - Bump `@metamask/gas-fee-controller` from `^26.2.4` to `^26.3.0` ([#9629](https://github.com/MetaMask/core/pull/9629)) @@ -25,13 +30,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [69.1.0] +### Added + +- Add `updateTransactionCallback` for atomic callback-based metadata updates and export `updateEIP7702BatchData` for synchronous indexed updates to nested transaction calldata ([#9543](https://github.com/MetaMask/core/pull/9543)) + ### Changed - Query layer 1 gas fee oracles via direct `eth_call` RPC requests instead of an ethers `Contract` backed by `Web3Provider` ([#9505](https://github.com/MetaMask/core/pull/9505)) - `Web3Provider` schedules its JSON-RPC dispatch with `setTimeout`, which never fires on React Native iOS when the timer pump is starved, blocking `addTransaction` indefinitely and preventing dapp confirmations from appearing ([MetaMask/metamask-mobile#32863](https://github.com/MetaMask/metamask-mobile/issues/32863)) -### Added - -- Add `updateTransactionCallback` for atomic callback-based metadata updates and export `updateEIP7702BatchData` for synchronous indexed updates to nested transaction calldata ([#9543](https://github.com/MetaMask/core/pull/9543)) ### Fixed diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index 03a698fdee7..d30b6c9b426 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `prepareTransactionAmount` support to `updateAmount` for atomically applying complete transaction amount patches and publishing one quote update ([#9543](https://github.com/MetaMask/core/pull/9543)) + ### Changed - Bump `@metamask/ramps-controller` from `^17.0.0` to `^17.1.0` ([#9646](https://github.com/MetaMask/core/pull/9646)) @@ -44,14 +48,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [25.1.0] +### Added + +- Add explicit `updateAmount` orchestration with complete-patch validation, in-flight intent deduplication, and synchronous coherent transaction updates before quote generation ([#9543](https://github.com/MetaMask/core/pull/9543)) + ### Changed - Consume `hasTransactionType` helper from `@metamask/transaction-controller` to derive relevant transaction type against the top-level `TransactionMeta` ([#9570](https://github.com/MetaMask/core/pull/9570)) - Bump `@metamask/transaction-controller` from `^69.0.0` to `^69.2.0` ([#9568](https://github.com/MetaMask/core/pull/9568), [#9589](https://github.com/MetaMask/core/pull/9589)) - Bump `@metamask/assets-controller` from `^11.0.0` to `^11.1.0` ([#9579](https://github.com/MetaMask/core/pull/9579)) -### Added - -- Add explicit `updateAmount` orchestration with complete-patch validation, in-flight intent deduplication, and synchronous coherent transaction updates before quote generation ([#9543](https://github.com/MetaMask/core/pull/9543)) ## [25.0.0] From f38dbfa14c84c5c63cfaf97c70cb6c5e69f04b9e Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Fri, 24 Jul 2026 15:09:01 +0100 Subject: [PATCH 10/11] fix: remove stale atomic update imports --- packages/transaction-controller/src/TransactionController.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 7534fa1676c..90492e8e71a 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -129,9 +129,6 @@ import type { AddTransactionOptions, PublishHookResult, GetGasFeeTokensRequest, - BeginAtomicBatchUpdateRequest, - BeginAtomicBatchUpdateResult, - AtomicBatchPreparationResult, } from './types.js'; import { GasFeeEstimateLevel, From 41234ae71d819541fdd1f2ea2b228800b196c57f Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Fri, 24 Jul 2026 15:21:52 +0100 Subject: [PATCH 11/11] test: cover transaction update branches --- .../src/TransactionController.test.ts | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 149eb4df58d..a6fd87ace7a 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -4960,6 +4960,27 @@ describe('TransactionController', () => { expect(result.txParams.value).toBe('0x2'); }); + it('uses a transaction returned by the callback', () => { + const { controller } = setupController({ + options: { + state: { + transactions: [TRANSACTION_META_MOCK], + }, + }, + }); + const updatedTransaction = { + ...TRANSACTION_META_MOCK, + txParams: { ...TRANSACTION_META_MOCK.txParams, value: '0x3' }, + }; + + const result = controller.updateTransactionCallback( + TRANSACTION_META_MOCK.id, + () => updatedTransaction, + ); + + expect(result.txParams.value).toBe('0x3'); + }); + it('throws if the transaction does not exist', () => { const { controller } = setupController(); @@ -7796,6 +7817,35 @@ describe('TransactionController', () => { expect(controller.state.transactions[0].gasLimitNoBuffer).toBe('0x200'); }); + it('clears stale revert metadata and applies a new gas revert', async () => { + updateGasMock.mockImplementationOnce(async ({ txMeta }) => { + txMeta.revert = { gas: { message: 'New gas revert' } }; + }); + const { controller } = setupController({ + options: { + state: { + transactions: [ + { + ...TRANSACTION_META_MOCK, + nestedTransactions: [{ to: ACCOUNT_2_MOCK, data: '0x1234' }], + revert: { gas: { message: 'Old gas revert' } }, + }, + ], + }, + }, + }); + + await controller.updateAtomicBatchData({ + transactionId: TRANSACTION_META_MOCK.id, + transactionIndex: 0, + transactionData: '0x89AB', + }); + + expect(controller.state.transactions[0].revert).toStrictEqual({ + gas: { message: 'New gas revert' }, + }); + }); + it('updates gas', async () => { const gasMock = '0x1234'; const gasLimitNoBufferMock = '0x123'; @@ -7836,6 +7886,26 @@ describe('TransactionController', () => { ).rejects.toThrow('Nested transaction not found'); }); + it('throws if the transaction has no nested transactions', async () => { + const { controller } = setupController({ + options: { + state: { + transactions: [ + { ...TRANSACTION_META_MOCK, nestedTransactions: undefined }, + ], + }, + }, + }); + + await expect( + controller.updateAtomicBatchData({ + transactionId: TRANSACTION_META_MOCK.id, + transactionIndex: 0, + transactionData: '0x89AB', + }), + ).rejects.toThrow('Nested transaction not found'); + }); + it('throws if batch transaction does not exist', async () => { const { controller } = setupController({ options: {