diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index a37416da197..71ac8ba26d4 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `updateTransactionCallback` for atomic callback-based metadata updates and export `updateEIP7702BatchData` for synchronous indexed updates to nested transaction calldata ([#9543](https://github.com/MetaMask/core/pull/9543)) + +### Fixed + +- 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-method-action-types.ts b/packages/transaction-controller/src/TransactionController-method-action-types.ts index ab865d28b14..ded8025ef6c 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. @@ -441,6 +453,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 a1c1d072381..e5c457eab12 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(); @@ -7683,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'; @@ -8374,6 +8487,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 b0ff4cd3533..29a85342183 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -141,9 +141,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 { @@ -697,6 +697,7 @@ const MESSENGER_EXPOSED_METHODS = [ 'updateSecurityAlertResponse', 'updateSelectedGasFeeToken', 'updateTransaction', + 'updateTransactionCallback', 'updateTransactionGasFees', 'wipeTransactions', ] as const; @@ -1618,6 +1619,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. @@ -2576,63 +2591,70 @@ export class TransactionController extends BaseController< transactionData, }); + const currentTransaction = this.#getTransaction(transactionId); + + if (!currentTransaction) { + throw new Error( + `Cannot update transaction as ID not found - ${transactionId}`, + ); + } + + const { nestedTransactions, transactionData: updatedTransactionData } = + updateEIP7702BatchData( + currentTransaction.txParams.from as Hex, + currentTransaction.nestedTransactions ?? [], + [{ transactionIndex, transactionData }], + ); const updatedTransactionMeta = this.#updateTransactionInternal( - { - transactionId, - }, + { transactionId }, (transactionMeta) => { - const { nestedTransactions, txParams } = transactionMeta; - const from = txParams.from as Hex; - const nestedTransaction = nestedTransactions?.[transactionIndex]; + 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 (!nestedTransaction) { - throw new Error( - `Nested transaction not found with index - ${transactionIndex}`, - ); + if ( + !transactionMeta.revert.simulation && + !transactionMeta.revert.receipt + ) { + transactionMeta.revert = undefined; + } } - - nestedTransaction.data = transactionData; - - const batchTransaction = generateEIP7702BatchTransaction( - from, - nestedTransactions, - ); - - transactionMeta.txParams.data = batchTransaction.data; }, ); - const draftTransaction = cloneDeep({ ...updatedTransactionMeta, txParams: { ...updatedTransactionMeta.txParams, - // Clear existing gas to force estimation + // Clear existing gas to force estimation. gas: undefined, }, }); await this.#updateGasEstimate(draftTransaction); - 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, - }; - } - }, - ); + 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 updatedTransactionMeta.txParams.data as Hex; + return updatedTransactionData; } /** diff --git a/packages/transaction-controller/src/index.ts b/packages/transaction-controller/src/index.ts index 9dbc08c9841..58d9f23540a 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, @@ -89,6 +90,7 @@ export type { Log, MetamaskPayMetadata, NestedTransactionMetadata, + NestedTransactionUpdate, PublishBatchHook, PublishBatchHookRequest, PublishBatchHookResult, @@ -132,6 +134,7 @@ export { mergeGasFeeEstimates } from './utils/gas-flow'; export { decodeAuthorizationSignature, generateEIP7702BatchTransaction, + updateEIP7702BatchData, } from './utils/eip7702'; export { isEIP1559Transaction, diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index 7fa97cdcc32..b395e0922e3 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -2331,6 +2331,15 @@ 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; +}; + /** * Decoded revert from a single lifecycle source. */ 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. * diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index ad877bfc9c5..24adffe7a64 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 synchronous coherent transaction updates before quote generation ([#9543](https://github.com/MetaMask/core/pull/9543)) + ## [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..ff5a3e72b48 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 for the updated transaction. + * 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..87ca6528eeb 100644 --- a/packages/transaction-pay-controller/src/TransactionPayController.test.ts +++ b/packages/transaction-pay-controller/src/TransactionPayController.test.ts @@ -1,7 +1,8 @@ /* eslint-disable no-new */ import type { TransactionMeta } from '@metamask/transaction-controller'; -import type { Hex } from '@metamask/utils'; +import type { Hex, Json } from '@metamask/utils'; +import { createDeferredPromise } from '@metamask/utils'; import { TransactionPayController } from '.'; import { updateFiatPayment } from './actions/update-fiat-payment'; @@ -10,13 +11,16 @@ 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'; -import { updateQuotes } from './utils/quotes'; +import { abortQuotes, updateQuotes } from './utils/quotes'; import { updateSourceAmounts } from './utils/source-amounts'; import { getTransaction, @@ -44,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, @@ -51,6 +56,7 @@ describe('TransactionPayController', () => { const subscribeAssetChangesMock = jest.mocked(subscribeAssetChanges); const getStrategyOrderMock = jest.mocked(getStrategyOrder); let messenger: TransactionPayControllerMessenger; + let updateTransactionCallbackMock: jest.Mock; let getKeyringControllerStateMock: jest.Mock; /** @@ -74,6 +80,7 @@ describe('TransactionPayController', () => { const mocks = getMessengerMock({ skipRegister: true }); messenger = mocks.messenger; + updateTransactionCallbackMock = mocks.updateTransactionCallbackMock; getKeyringControllerStateMock = mocks.getKeyringControllerStateMock; getKeyringControllerStateMock.mockReturnValue({ @@ -106,6 +113,426 @@ 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 mockTransactionUpdateCallback(): TransactionMeta { + const updatedTransaction = { + ...transaction, + nestedTransactions: transaction.nestedTransactions?.map( + (nestedTransaction) => ({ ...nestedTransaction }), + ), + txParams: { ...transaction.txParams }, + }; + updateTransactionCallbackMock.mockImplementation( + (_transactionId, callback) => { + callback(updatedTransaction); + return updatedTransaction; + }, + ); + return updatedTransaction; + } + + 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('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(updateTransactionCallbackMock).not.toHaveBeenCalled(); + }); + + 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); + const updatedTransaction = mockTransactionUpdateCallback(); + 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(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.mock.calls[0][0]).not.toHaveProperty( + 'transactionPreparation', + ); + expect(updateQuotesMock.mock.calls[0][0]).not.toHaveProperty( + 'transactionRevision', + ); + }); + + 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(updateTransactionCallbackMock).not.toHaveBeenCalled(); + expect(updateQuotesMock).not.toHaveBeenCalled(); + }); + + 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({ + kind: 'prepared', + amountRaw: '1250000', + requiredAssets, + nestedTransactionUpdates, + requiredNestedTransactionIndexes: [0, 1], + }), + state: getStateWithOldQuote(), + }); + getTransactionMock.mockReturnValue(transaction); + mockTransactionUpdateCallback(); + 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); + mockTransactionUpdateCallback(); + + 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); + mockTransactionUpdateCallback(); + 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); + mockTransactionUpdateCallback(); + 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 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', + 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(updateTransactionCallbackMock).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 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, + 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..ee2a48f36be 100644 --- a/packages/transaction-pay-controller/src/TransactionPayController.ts +++ b/packages/transaction-pay-controller/src/TransactionPayController.ts @@ -1,6 +1,8 @@ import type { StateMetadata } from '@metamask/base-controller'; import { BaseController } from '@metamask/base-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'; @@ -17,19 +19,23 @@ import type { GetDelegationTransactionCallback, GetPaymentOverrideDataCallback, PolymarketCallbacks, + PrepareTransactionAmountCallback, + PrepareTransactionAmountResult, TransactionConfigCallback, TransactionData, TransactionPayControllerMessenger, TransactionPayFiatOptions, TransactionPayControllerOptions, TransactionPayControllerState, + UpdateAmountRequest, UpdateFiatPaymentRequest, 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, subscribeAssetChanges, subscribeTransactionChanges, } from './utils/transaction'; @@ -43,6 +49,7 @@ const MESSENGER_EXPOSED_METHODS = [ 'polymarketGetDepositWalletAddress', 'polymarketSubmitDepositWalletBatch', 'setTransactionConfig', + 'updateAmount', 'updateFiatPayment', 'updatePaymentToken', ] as const; @@ -83,6 +90,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 +112,7 @@ export class TransactionPayController extends BaseController< getStrategies, messenger, polymarket, + prepareTransactionAmount, state, }: TransactionPayControllerOptions) { super({ @@ -108,6 +129,7 @@ export class TransactionPayController extends BaseController< this.#getStrategy = getStrategy; this.#getStrategies = getStrategies; this.#polymarket = polymarket; + this.#prepareTransactionAmount = prepareTransactionAmount; this.messenger.registerMethodActionHandlers( this, @@ -182,6 +204,156 @@ export class TransactionPayController extends BaseController< }); } + /** + * Prepares and atomically commits an exact transaction amount, then launches + * one quote generation for the updated transaction. + * 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; + }); + abortQuotes(transactionId); + + 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'); + } + + this.#updateTransactionAmount(transactionId, amountPreparation); + + return await updateQuotes({ + getStrategies: this.#getStrategiesWithFallback.bind(this), + messenger: this.messenger, + signal, + transactionData: this.state.transactionData[transactionId], + transactionId, + updateTransactionData: this.#updateTransactionData.bind(this), + }); + } + + #updateTransactionAmount( + transactionId: string, + amountPreparation: Extract< + PrepareTransactionAmountResult, + { kind: 'prepared' } + >, + ): void { + this.#quoteSuppressedTransactionIds.add(transactionId); + + try { + 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 { + 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 +557,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/tests/messenger-mock.ts b/packages/transaction-pay-controller/src/tests/messenger-mock.ts index 4e3b1897c45..6d596389361 100644 --- a/packages/transaction-pay-controller/src/tests/messenger-mock.ts +++ b/packages/transaction-pay-controller/src/tests/messenger-mock.ts @@ -17,6 +17,7 @@ import type { TransactionControllerAddTransactionAction, TransactionControllerAddTransactionBatchAction, TransactionControllerEstimateGasAction, + TransactionControllerUpdateTransactionCallbackAction, TransactionControllerEstimateGasBatchAction, TransactionControllerGetGasFeeTokensAction, TransactionControllerGetStateAction, @@ -69,6 +70,10 @@ export function getMessengerMock({ TransactionControllerAddTransactionBatchAction['handler'] > = jest.fn(); + const updateTransactionCallbackMock: jest.MockedFn< + TransactionControllerUpdateTransactionCallbackAction['handler'] + > = jest.fn(); + const findNetworkClientIdByChainIdMock: jest.MockedFn< NetworkControllerFindNetworkClientIdByChainIdAction['handler'] > = jest.fn(); @@ -287,6 +292,11 @@ export function getMessengerMock({ ); } + messenger.registerActionHandler( + 'TransactionController:updateTransactionCallback', + updateTransactionCallbackMock, + ); + messenger.registerActionHandler( 'KeyringController:getState', getKeyringControllerStateMock, @@ -322,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 c1904e38f0d..8798b353120 100644 --- a/packages/transaction-pay-controller/src/types.ts +++ b/packages/transaction-pay-controller/src/types.ts @@ -35,6 +35,8 @@ import type { import type { RemoteFeatureFlagControllerGetStateAction } from '@metamask/remote-feature-flag-controller'; import type { AuthorizationList, + NestedTransactionUpdate, + RequiredAsset, TransactionControllerAddTransactionBatchAction, TransactionControllerEstimateGasAction, TransactionControllerEstimateGasBatchAction, @@ -48,6 +50,7 @@ import type { TransactionControllerGetStateAction, TransactionControllerStateChangeEvent, TransactionControllerUpdateTransactionAction, + TransactionControllerUpdateTransactionCallbackAction, TransactionMeta, } from '@metamask/transaction-controller'; import type { Hex, Json } from '@metamask/utils'; @@ -82,7 +85,8 @@ export type AllowedActions = | TransactionControllerEstimateGasBatchAction | TransactionControllerGetGasFeeTokensAction | TransactionControllerGetStateAction - | TransactionControllerUpdateTransactionAction; + | TransactionControllerUpdateTransactionAction + | TransactionControllerUpdateTransactionCallbackAction; export type AllowedEvents = | AssetsControllerStateChangeEvent @@ -201,6 +205,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 +293,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; diff --git a/packages/transaction-pay-controller/src/utils/quotes.test.ts b/packages/transaction-pay-controller/src/utils/quotes.test.ts index f414b8fe2fc..1f251ae6e01 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.test.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.test.ts @@ -15,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, @@ -198,6 +198,18 @@ describe('Quotes Utils', () => { }); describe('updateQuotes', () => { + 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('updates quotes in state', async () => { await run(); @@ -1039,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 7ae137bdc7f..d9db26b9001 100644 --- a/packages/transaction-pay-controller/src/utils/quotes.ts +++ b/packages/transaction-pay-controller/src/utils/quotes.ts @@ -1,6 +1,8 @@ import { TransactionStatus } from '@metamask/transaction-controller'; -import type { BatchTransaction } from '@metamask/transaction-controller'; -import type { TransactionMeta } from '@metamask/transaction-controller'; +import type { + BatchTransaction, + TransactionMeta, +} from '@metamask/transaction-controller'; import type { Hex, Json } from '@metamask/utils'; import { createModuleLogger } from '@metamask/utils'; @@ -42,6 +44,7 @@ const inFlightQuoteRequests = new Map(); export type UpdateQuotesRequest = { getStrategies: (transaction: TransactionMeta) => TransactionPayStrategy[]; messenger: TransactionPayControllerMessenger; + signal?: AbortSignal; transactionData: TransactionData | undefined; transactionId: string; updateTransactionData: UpdateTransactionDataCallback; @@ -64,6 +67,7 @@ export async function updateQuotes( const { getStrategies, messenger, + signal: externalSignal, transactionData, transactionId, updateTransactionData, @@ -100,6 +104,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; @@ -200,6 +214,7 @@ export async function updateQuotes( data.isLoading = false; }); } + externalSignal?.removeEventListener('abort', abortFromExternalSignal); clearControllerIfCurrent(transactionId, controller); } @@ -340,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);