From 9a2175e84a4c7eda3761dc10236a669c6c3daf13 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 16 Jul 2026 17:07:11 +0100 Subject: [PATCH 1/8] feat: optimize transaction pay amount quote pipeline --- packages/transaction-controller/CHANGELOG.md | 4 + ...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 | 4 + ...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, 1229 insertions(+), 43 deletions(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index a37416da197..ff019932c78 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `beginAtomicBatchUpdate` for coherent multi-call updates with monotonic revisions and revision-bound gas preparation + ## [69.0.0] ### Changed diff --git a/packages/transaction-controller/src/TransactionController-method-action-types.ts b/packages/transaction-controller/src/TransactionController-method-action-types.ts index ab865d28b14..f9e2860cd4e 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 a1c1d072381..00fab65b530 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 b0ff4cd3533..c8c8795c135 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'; import { GasFeeEstimateLevel, @@ -671,6 +674,7 @@ const MESSENGER_EXPOSED_METHODS = [ 'addTransaction', 'addTransactionBatch', 'approveTransactionsWithSameNonce', + 'beginAtomicBatchUpdate', 'clearUnapprovedTransactions', 'confirmExternalTransaction', 'emulateNewTransaction', @@ -2552,6 +2556,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. * @@ -2576,46 +2693,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; @@ -2632,7 +2753,11 @@ export class TransactionController extends BaseController< }, ); - return updatedTransactionMeta.txParams.data as Hex; + return { + revision, + status: 'prepared', + transaction: cloneDeep(preparedTransaction), + }; } /** @@ -4121,6 +4246,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 9dbc08c9841..1078b5c3705 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 7fa97cdcc32..c1b2159d333 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 ad877bfc9c5..45ad7e6dde0 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 explicit `updateAmount` orchestration with complete-patch validation, in-flight intent deduplication, and revision-bound Relay quote publication + ## [25.0.0] ### Added 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 ff753bb1792..f3b5a940353 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 fcb6c4e840e..94a6fff6b8a 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 { TransactionPayController } from '.'; import { updateFiatPayment } from './actions/update-fiat-payment'; @@ -10,9 +14,12 @@ import { PaymentOverride, TransactionPayStrategy } from './constants'; import { deriveFiatAssetForFiatPayment } from './strategy/fiat/utils'; import { getMessengerMock } from './tests/messenger-mock'; import type { + PrepareTransactionAmountResult, TransactionPayControllerMessenger, TransactionPayControllerOptions, + TransactionPayQuote, TransactionPaySourceAmount, + TransactionPayTotals, UpdateTransactionDataCallback, } from './types'; import { getStrategyOrder } from './utils/feature-flags'; @@ -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 c9e31d480d1..8c94fc7abb9 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'; @@ -30,6 +36,7 @@ import { getStrategyOrder } from './utils/feature-flags'; import { updateQuotes } from './utils/quotes'; import { updateSourceAmounts } from './utils/source-amounts'; import { + getTransaction, subscribeAssetChanges, subscribeTransactionChanges, } from './utils/transaction'; @@ -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 dbab894b8ac..fbaea534219 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, @@ -23,6 +26,7 @@ export type { TransactionPayRequiredToken, TransactionPaySourceAmount, TransactionPayTotals, + UpdateAmountRequest, UpdateFiatPaymentRequest, UpdatePaymentTokenRequest, } from './types'; @@ -34,6 +38,7 @@ export type { TransactionPayControllerPolymarketGetDepositWalletAddressAction, TransactionPayControllerPolymarketSubmitDepositWalletBatchAction, TransactionPayControllerSetTransactionConfigAction, + TransactionPayControllerUpdateAmountAction, TransactionPayControllerUpdatePaymentTokenAction, TransactionPayControllerUpdateFiatPaymentAction, } from './TransactionPayController-method-action-types'; 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 cd6ffa21974..b6b89806fcc 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'; @@ -244,6 +246,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 470b0fd2d76..f1b2ef2177c 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts @@ -352,7 +352,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 4e3b1897c45..6bd9f87b251 100644 --- a/packages/transaction-pay-controller/src/tests/messenger-mock.ts +++ b/packages/transaction-pay-controller/src/tests/messenger-mock.ts @@ -16,6 +16,7 @@ import type { RemoteFeatureFlagControllerGetStateAction } from '@metamask/remote import type { TransactionControllerAddTransactionAction, TransactionControllerAddTransactionBatchAction, + TransactionControllerBeginAtomicBatchUpdateAction, TransactionControllerEstimateGasAction, TransactionControllerEstimateGasBatchAction, TransactionControllerGetGasFeeTokensAction, @@ -69,6 +70,10 @@ export function getMessengerMock({ TransactionControllerAddTransactionBatchAction['handler'] > = jest.fn(); + const beginAtomicBatchUpdateMock: jest.MockedFn< + TransactionControllerBeginAtomicBatchUpdateAction['handler'] + > = jest.fn(); + const findNetworkClientIdByChainIdMock: jest.MockedFn< NetworkControllerFindNetworkClientIdByChainIdAction['handler'] > = jest.fn(); @@ -287,6 +292,11 @@ export function getMessengerMock({ ); } + messenger.registerActionHandler( + 'TransactionController:beginAtomicBatchUpdate', + beginAtomicBatchUpdateMock, + ); + messenger.registerActionHandler( 'KeyringController:getState', getKeyringControllerStateMock, @@ -296,6 +306,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 c1904e38f0d..2114ee1b87b 100644 --- a/packages/transaction-pay-controller/src/types.ts +++ b/packages/transaction-pay-controller/src/types.ts @@ -34,8 +34,12 @@ import type { } from '@metamask/ramps-controller'; import type { RemoteFeatureFlagControllerGetStateAction } from '@metamask/remote-feature-flag-controller'; import type { + AtomicBatchPreparationResult, AuthorizationList, + NestedTransactionUpdate, + RequiredAsset, TransactionControllerAddTransactionBatchAction, + TransactionControllerBeginAtomicBatchUpdateAction, TransactionControllerEstimateGasAction, TransactionControllerEstimateGasBatchAction, TransactionControllerUnapprovedTransactionAddedEvent, @@ -78,6 +82,7 @@ export type AllowedActions = | TokensControllerGetStateAction | TransactionControllerAddTransactionAction | TransactionControllerAddTransactionBatchAction + | TransactionControllerBeginAtomicBatchUpdateAction | TransactionControllerEstimateGasAction | TransactionControllerEstimateGasBatchAction | TransactionControllerGetGasFeeTokensAction @@ -201,6 +206,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, @@ -240,6 +294,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; @@ -630,6 +687,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 f414b8fe2fc..e0efcdeab45 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'; @@ -198,6 +202,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 7ae137bdc7f..82c4dc408b6 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'; @@ -42,8 +45,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; }; @@ -64,8 +70,11 @@ export async function updateQuotes( const { getStrategies, messenger, + signal: externalSignal, transactionData, transactionId, + transactionPreparation, + transactionRevision, updateTransactionData, } = request; @@ -100,6 +109,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; @@ -147,6 +166,7 @@ export async function updateQuotes( messenger, fiatPayment?.selectedPaymentMethodId, signal, + transactionPreparation, ); if (signal.aborted) { @@ -154,6 +174,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". @@ -167,7 +212,7 @@ export async function updateQuotes( messenger, quotes: executableQuotes as TransactionPayQuote[], tokens, - transaction, + transaction: preparedTransaction, }); log('Calculated totals', { transactionId, totals }); @@ -200,6 +245,7 @@ export async function updateQuotes( data.isLoading = false; }); } + externalSignal?.removeEventListener('abort', abortFromExternalSignal); clearControllerIfCurrent(transactionId, controller); } @@ -615,6 +661,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( @@ -628,6 +675,7 @@ async function getQuotes( messenger: TransactionPayControllerMessenger, fiatPaymentMethod?: string, signal?: AbortSignal, + transactionPreparation?: Promise, ): Promise<{ batchTransactions: BatchTransaction[]; quotes: TransactionPayQuote[]; @@ -672,6 +720,7 @@ async function getQuotes( requests, signal, transaction, + transactionPreparation, }; for (const { name, strategy } of strategies) { From 732f0f730197750208ae14a2fa5a5f6b88927712 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 16 Jul 2026 17:09:29 +0100 Subject: [PATCH 2/8] 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 ff019932c78..94ef84ffa62 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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 45ad7e6dde0..df03678e9a9 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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 54520d254046409de2398fe5624528392d110e38 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 16 Jul 2026 17:48:20 +0100 Subject: [PATCH 3/8] 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 94a6fff6b8a..185f53f3cde 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 b6b89806fcc..40e3d91dc90 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 @@ -280,6 +280,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 e0efcdeab45..b17eea016f0 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.test.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.test.ts @@ -236,6 +236,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 670fa768ecf0f0a78deb67ffc50f049357f816d5 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Wed, 22 Jul 2026 11:56:36 +0100 Subject: [PATCH 4/8] 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 00fab65b530..f972d2d82d5 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 c8c8795c135..27a7d96d720 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -2716,12 +2716,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); @@ -2753,6 +2758,10 @@ export class TransactionController extends BaseController< }, ); + if (simulationPreparationResult.status === 'rejected') { + throw simulationPreparationResult.reason; + } + return { revision, status: 'prepared', From a5d05c5d21e5e119ecd15e8ead05b9cdcd04a840 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Wed, 22 Jul 2026 12:06:48 +0100 Subject: [PATCH 5/8] 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 94ef84ffa62..a89edf06698 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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 27a7d96d720..0df435ba654 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'; import { addTransactionBatch, isAtomicBatchSupported } from './utils/batch'; import { - generateEIP7702BatchTransaction, getDelegationAddress, signAuthorizationList, + updateEIP7702BatchData, } from './utils/eip7702'; import { validateConfirmedExternalTransaction } from './utils/external-transactions'; import { @@ -2582,56 +2582,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 1078b5c3705..d277341c705 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'; export { decodeAuthorizationSignature, generateEIP7702BatchTransaction, + updateEIP7702BatchData, } from './utils/eip7702'; export { isEIP1559Transaction, diff --git a/packages/transaction-controller/src/utils/eip7702.test.ts b/packages/transaction-controller/src/utils/eip7702.test.ts index 155eb007ee1..47de1fe7e76 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'; 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 91b39c390de..97bd8c9c45f 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'; import type { TransactionControllerMessenger } from '../TransactionController'; 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 836ecc49657b61c84b78525f4b7dbca87eaadae7 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Wed, 22 Jul 2026 12:16:26 +0100 Subject: [PATCH 6/8] 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 a89edf06698..ced8119106c 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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 f9e2860cd4e..31d71b36847 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 f972d2d82d5..7d2f29de843 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 0df435ba654..659ffc4f1d3 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -701,6 +701,7 @@ const MESSENGER_EXPOSED_METHODS = [ 'updateSecurityAlertResponse', 'updateSelectedGasFeeToken', 'updateTransaction', + 'updateTransactionCallback', 'updateTransactionGasFees', 'wipeTransactions', ] as const; @@ -1622,6 +1623,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 d277341c705..7c4aabc285c 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 411a67f5560f13f3cf528598167009f0e4f650e8 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Wed, 22 Jul 2026 12:50:26 +0100 Subject: [PATCH 7/8] 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 | 189 +++---------- 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(+), 819 deletions(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index ced8119106c..011aa26438b 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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 31d71b36847..ded8025ef6c 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 7d2f29de843..815a9ea5142 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 659ffc4f1d3..373e41efaa7 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'; import { GasFeeEstimateLevel, @@ -674,7 +671,6 @@ const MESSENGER_EXPOSED_METHODS = [ 'addTransaction', 'addTransactionBatch', 'approveTransactionsWithSameNonce', - 'beginAtomicBatchUpdate', 'clearUnapprovedTransactions', 'confirmExternalTransaction', 'emulateNewTransaction', @@ -2571,85 +2567,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. * @@ -2682,72 +2599,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; } /** @@ -4236,17 +4126,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 7c4aabc285c..58d9f23540a 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 c1b2159d333..b395e0922e3 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 df03678e9a9..24adffe7a64 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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 f3b5a940353..ff5a3e72b48 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 185f53f3cde..87ca6528eeb 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'; import { getStrategyOrder } from './utils/feature-flags'; -import { updateQuotes } from './utils/quotes'; +import { abortQuotes, updateQuotes } from './utils/quotes'; import { updateSourceAmounts } from './utils/source-amounts'; 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 8c94fc7abb9..ee2a48f36be 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'; import { getStrategyOrder } from './utils/feature-flags'; -import { updateQuotes } from './utils/quotes'; +import { abortQuotes, updateQuotes } from './utils/quotes'; import { updateSourceAmounts } from './utils/source-amounts'; 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 40e3d91dc90..cd6ffa21974 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'; @@ -246,64 +244,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 f1b2ef2177c..470b0fd2d76 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts @@ -352,26 +352,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 6bd9f87b251..6d596389361 100644 --- a/packages/transaction-pay-controller/src/tests/messenger-mock.ts +++ b/packages/transaction-pay-controller/src/tests/messenger-mock.ts @@ -16,8 +16,8 @@ import type { RemoteFeatureFlagControllerGetStateAction } from '@metamask/remote import type { TransactionControllerAddTransactionAction, TransactionControllerAddTransactionBatchAction, - TransactionControllerBeginAtomicBatchUpdateAction, TransactionControllerEstimateGasAction, + TransactionControllerUpdateTransactionCallbackAction, TransactionControllerEstimateGasBatchAction, TransactionControllerGetGasFeeTokensAction, TransactionControllerGetStateAction, @@ -70,8 +70,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< @@ -293,8 +293,8 @@ export function getMessengerMock({ } messenger.registerActionHandler( - 'TransactionController:beginAtomicBatchUpdate', - beginAtomicBatchUpdateMock, + 'TransactionController:updateTransactionCallback', + updateTransactionCallbackMock, ); messenger.registerActionHandler( @@ -306,7 +306,6 @@ export function getMessengerMock({ return { addTransactionMock, - beginAtomicBatchUpdateMock, getAssetsControllerStateMock, addTransactionBatchMock, estimateGasMock, @@ -333,6 +332,7 @@ export function getMessengerMock({ polymarketGetDepositWalletAddressMock, polymarketSubmitDepositWalletBatchMock, publish, + updateTransactionCallbackMock, updateTransactionMock, }; } diff --git a/packages/transaction-pay-controller/src/types.ts b/packages/transaction-pay-controller/src/types.ts index 2114ee1b87b..8798b353120 100644 --- a/packages/transaction-pay-controller/src/types.ts +++ b/packages/transaction-pay-controller/src/types.ts @@ -34,12 +34,10 @@ import type { } from '@metamask/ramps-controller'; import type { RemoteFeatureFlagControllerGetStateAction } from '@metamask/remote-feature-flag-controller'; import type { - AtomicBatchPreparationResult, AuthorizationList, NestedTransactionUpdate, RequiredAsset, TransactionControllerAddTransactionBatchAction, - TransactionControllerBeginAtomicBatchUpdateAction, TransactionControllerEstimateGasAction, TransactionControllerEstimateGasBatchAction, TransactionControllerUnapprovedTransactionAddedEvent, @@ -52,6 +50,7 @@ import type { TransactionControllerGetStateAction, TransactionControllerStateChangeEvent, TransactionControllerUpdateTransactionAction, + TransactionControllerUpdateTransactionCallbackAction, TransactionMeta, } from '@metamask/transaction-controller'; import type { Hex, Json } from '@metamask/utils'; @@ -82,12 +81,12 @@ export type AllowedActions = | TokensControllerGetStateAction | TransactionControllerAddTransactionAction | TransactionControllerAddTransactionBatchAction - | TransactionControllerBeginAtomicBatchUpdateAction | TransactionControllerEstimateGasAction | TransactionControllerEstimateGasBatchAction | TransactionControllerGetGasFeeTokensAction | TransactionControllerGetStateAction - | TransactionControllerUpdateTransactionAction; + | TransactionControllerUpdateTransactionAction + | TransactionControllerUpdateTransactionCallbackAction; export type AllowedEvents = | AssetsControllerStateChangeEvent @@ -687,9 +686,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 b17eea016f0..1f251ae6e01 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'; @@ -19,7 +15,7 @@ import type { TransactionPayRequiredToken, } from '../types'; import type { UpdateQuotesRequest } from './quotes'; -import { refreshQuotes, updateQuotes } from './quotes'; +import { abortQuotes, refreshQuotes, updateQuotes } from './quotes'; import { checkStrategyQuoteSupport, checkStrategySupport, @@ -202,97 +198,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(); @@ -305,50 +210,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(); @@ -1190,6 +1051,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 82c4dc408b6..d9db26b9001 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'; @@ -48,8 +47,6 @@ export type UpdateQuotesRequest = { signal?: AbortSignal; transactionData: TransactionData | undefined; transactionId: string; - transactionPreparation?: Promise; - transactionRevision?: number; updateTransactionData: UpdateTransactionDataCallback; }; @@ -73,8 +70,6 @@ export async function updateQuotes( signal: externalSignal, transactionData, transactionId, - transactionPreparation, - transactionRevision, updateTransactionData, } = request; @@ -166,7 +161,6 @@ export async function updateQuotes( messenger, fiatPayment?.selectedPaymentMethodId, signal, - transactionPreparation, ); if (signal.aborted) { @@ -174,31 +168,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". @@ -212,7 +181,7 @@ export async function updateQuotes( messenger, quotes: executableQuotes as TransactionPayQuote[], tokens, - transaction: preparedTransaction, + transaction, }); log('Calculated totals', { transactionId, totals }); @@ -386,15 +355,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); @@ -661,7 +639,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( @@ -675,7 +652,6 @@ async function getQuotes( messenger: TransactionPayControllerMessenger, fiatPaymentMethod?: string, signal?: AbortSignal, - transactionPreparation?: Promise, ): Promise<{ batchTransactions: BatchTransaction[]; quotes: TransactionPayQuote[]; @@ -720,7 +696,6 @@ async function getQuotes( requests, signal, transaction, - transactionPreparation, }; for (const { name, strategy } of strategies) { From 366f7b73d3215a8e065f1f841ce8ee82f47cd235 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Wed, 22 Jul 2026 13:36:20 +0100 Subject: [PATCH 8/8] 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 011aa26438b..71ac8ba26d4 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -11,6 +11,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 815a9ea5142..e5c457eab12 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 373e41efaa7..29a85342183 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -2610,6 +2610,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({