From 9a2175e84a4c7eda3761dc10236a669c6c3daf13 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Thu, 16 Jul 2026 17:07:11 +0100 Subject: [PATCH 1/3] 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/3] 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/3] 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();