diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index f687e007ed7..633c5f124c8 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add `instant` option to `addTransaction` that adds the transaction to state immediately with default values, deferring gas estimation, fee calculation, and type resolution to the background ([#XXXX](https://github.com/MetaMask/core/pull/XXXX)) +- Add `ready` property to `TransactionMeta` that indicates whether essential async data has been resolved for instant transactions ([#XXXX](https://github.com/MetaMask/core/pull/XXXX)) +- Add safety guards that prevent approving, speeding up, or cancelling transactions when `ready` is `false` ([#XXXX](https://github.com/MetaMask/core/pull/XXXX)) + +## [62.22.0] + +### Added + - Add optional `sourceHash` field to `MetamaskPayMetadata` for tracking source chain transaction hashes when no local transaction exists ([#8133](https://github.com/MetaMask/core/pull/8133)) - Add `predictDepositAndOrder` to `TransactionType` ([#8135](https://github.com/MetaMask/core/pull/8135)) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index 9cb6ea3e0bc..20c56b06975 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -3497,6 +3497,106 @@ describe('TransactionController', () => { await expect(result).rejects.toThrow('Unknown problem'); }); }); + + describe('ready gate', () => { + it('fails the transaction when ready is false', async () => { + const { controller } = setupController({ + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, + }); + + const { result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x0', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + instant: true, + }, + ); + + await expect(result).rejects.toThrow( + 'Transaction is not ready. Essential async data has not resolved yet.', + ); + + expect(controller.state.transactions[0]).toMatchObject( + expect.objectContaining({ + status: TransactionStatus.failed, + }), + ); + }); + + it('proceeds normally when ready is true', async () => { + const { controller, mockTransactionApprovalRequest } = + setupController(); + + const { result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x0', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + instant: true, + }, + ); + + // Let background resolution complete, setting ready to true + await flushPromises(); + + // Now approve the transaction + mockTransactionApprovalRequest.approve(); + + await result; + + expect(controller.state.transactions[0]).toMatchObject( + expect.objectContaining({ + status: TransactionStatus.submitted, + }), + ); + }); + + it('proceeds normally when ready is undefined', async () => { + const { controller } = setupController({ + messengerOptions: { + addTransactionApprovalRequest: { + state: 'approved', + }, + }, + }); + + const { result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x0', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); + + await result; + + expect(controller.state.transactions[0]).toMatchObject( + expect.objectContaining({ + status: TransactionStatus.submitted, + }), + ); + }); + }); }); describe('on reject', () => { @@ -3853,6 +3953,256 @@ describe('TransactionController', () => { }); }); }); + + describe('when instant option is true', () => { + it('returns immediately with ready false', async () => { + const { controller } = setupController(); + + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + instant: true, + }, + ); + + expect(transactionMeta.ready).toBe(false); + expect(transactionMeta.status).toBe(TransactionStatus.unapproved); + }); + + it('adds transaction to state immediately', async () => { + const { controller } = setupController(); + + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + instant: true, + }, + ); + + const txInState = controller.state.transactions.find( + (tx) => tx.id === transactionMeta.id, + ); + + expect(txInState).toBeDefined(); + expect(txInState?.ready).toBe(false); + }); + + it('sets ready to true after background resolution', async () => { + const { controller } = setupController(); + + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + instant: true, + }, + ); + + await flushPromises(); + + const txInState = controller.state.transactions.find( + (tx) => tx.id === transactionMeta.id, + ); + + expect(txInState?.ready).toBe(true); + }); + + it('throws if instant is true with an external origin', async () => { + const { controller } = setupController(); + + await expect( + controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + instant: true, + origin: 'https://metamask.github.io/test-dapp/', + }, + ), + ).rejects.toThrow( + 'The instant option is not supported for external transactions.', + ); + }); + + it('does not throw if instant is true with ORIGIN_METAMASK', async () => { + const { controller } = setupController(); + + await expect( + controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + instant: true, + origin: ORIGIN_METAMASK, + }, + ), + ).resolves.toMatchObject({ + transactionMeta: expect.objectContaining({ ready: false }), + }); + }); + + it('publishes unapprovedTransactionAdded event immediately', async () => { + const { controller, messenger } = setupController(); + const listener = jest.fn(); + + messenger.subscribe( + 'TransactionController:unapprovedTransactionAdded', + listener, + ); + + await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + instant: true, + }, + ); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener.mock.calls[0][0]).toMatchObject({ ready: false }); + }); + + it('does not set ready when instant is not true', async () => { + const { controller } = setupController(); + + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); + + expect(transactionMeta.ready).toBeUndefined(); + }); + + it('uses caller-provided type without waiting for background', async () => { + const { controller } = setupController(); + + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + instant: true, + type: TransactionType.simpleSend, + }, + ); + + expect(transactionMeta.type).toBe(TransactionType.simpleSend); + }); + + it('works with requireApproval false', async () => { + const { controller } = setupController(); + + const { transactionMeta, result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + instant: true, + requireApproval: false, + }, + ); + + result.catch(() => { + // Expected: ready gate rejects because ready is false + }); + + expect(transactionMeta.ready).toBe(false); + expect( + controller.state.transactions.some( + (tx) => tx.id === transactionMeta.id, + ), + ).toBe(true); + }); + + it('adds multiple instant transactions in quick succession', async () => { + uuidModuleMock.v1 + .mockImplementationOnce(() => 'aaa-instant-1') + .mockImplementationOnce(() => 'bbb-instant-2') + .mockImplementationOnce(() => 'ccc-instant-3'); + + const { controller } = setupController(); + + const results = await Promise.all([ + controller.addTransaction( + { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK }, + { networkClientId: NETWORK_CLIENT_ID_MOCK, instant: true }, + ), + controller.addTransaction( + { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK }, + { networkClientId: NETWORK_CLIENT_ID_MOCK, instant: true }, + ), + controller.addTransaction( + { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK }, + { networkClientId: NETWORK_CLIENT_ID_MOCK, instant: true }, + ), + ]); + + const ids = results.map((r) => r.transactionMeta.id); + + expect(new Set(ids).size).toBe(3); + expect( + controller.state.transactions.filter((tx) => ids.includes(tx.id)), + ).toHaveLength(3); + }); + + it('fails the transaction if background resolution throws', async () => { + updateGasMock.mockImplementationOnce(() => { + throw new Error('gas estimation failed'); + }); + + const { controller } = setupController(); + + const { transactionMeta, result } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + instant: true, + }, + ); + + result.catch(() => undefined); + + await flushPromises(); + + const txInState = controller.state.transactions.find( + (tx) => tx.id === transactionMeta.id, + ); + + expect(txInState?.status).toBe(TransactionStatus.failed); + }); + }); }); describe('wipeTransactions', () => { @@ -4333,6 +4683,51 @@ describe('TransactionController', () => { }), ]); }); + + describe('when ready is false', () => { + it('throws when transaction has ready false', async () => { + const { controller } = setupController(); + + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + instant: true, + }, + ); + + await expect( + controller.stopTransaction(transactionMeta.id), + ).rejects.toThrow( + 'Cannot cancel transaction: essential async data has not resolved yet.', + ); + }); + + it('does not throw ready error when ready is undefined', async () => { + const { controller } = setupController(); + + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); + + try { + await controller.stopTransaction(transactionMeta.id); + } catch (error: unknown) { + expect((error as Error).message).not.toBe( + 'Cannot cancel transaction: essential async data has not resolved yet.', + ); + } + }); + }); }); describe('speedUpTransaction', () => { @@ -4761,6 +5156,51 @@ describe('TransactionController', () => { }), ]); }); + + describe('when ready is false', () => { + it('throws when transaction has ready false', async () => { + const { controller } = setupController(); + + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + instant: true, + }, + ); + + await expect( + controller.speedUpTransaction(transactionMeta.id), + ).rejects.toThrow( + 'Cannot speed up transaction: essential async data has not resolved yet.', + ); + }); + + it('does not throw ready error when ready is undefined', async () => { + const { controller } = setupController(); + + const { transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + to: ACCOUNT_MOCK, + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); + + try { + await controller.speedUpTransaction(transactionMeta.id); + } catch (error: unknown) { + expect((error as Error).message).not.toBe( + 'Cannot speed up transaction: essential async data has not resolved yet.', + ); + } + }); + }); }); describe('confirmExternalTransaction', () => { diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index ed82765de80..b21b9b09d8f 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -1290,6 +1290,7 @@ export class TransactionController extends BaseController< requireApproval, securityAlertResponse, skipInitialGasEstimate, + instant, swaps = {}, traceContext, type, @@ -1302,6 +1303,72 @@ export class TransactionController extends BaseController< throw new Error(`Network client not found - ${networkClientId}`); } + if (instant && origin !== undefined && origin !== ORIGIN_METAMASK) { + throw new Error( + 'The instant option is not supported for external transactions.', + ); + } + + if (instant) { + const chainId = this.#getChainId(networkClientId); + const dappSuggestedGasFees = this.#generateDappSuggestedGasFees( + txParams, + origin, + ); + + const instantTransactionMeta: TransactionMeta = { + actionId, + assetsFiatValues, + batchId, + chainId, + dappSuggestedGasFees, + deviceConfirmedOn, + disableGasBuffer, + id: random(), + isGasFeeTokenIgnoredIfBalance: Boolean(gasFeeToken), + isGasFeeIncluded, + isGasFeeSponsored, + isFirstTimeInteraction: undefined, + isStateOnly, + nestedTransactions, + networkClientId, + origin, + ready: false, + requestId, + requiredAssets, + securityAlertResponse, + selectedGasFeeToken: gasFeeToken, + status: TransactionStatus.unapproved as const, + time: Date.now(), + txParams, + type, + userEditedGasLimit: false, + verifiedOnBlockchain: false, + }; + + this.#addMetadata(instantTransactionMeta); + this.#resolveInstantTransaction( + cloneDeep(instantTransactionMeta), + options, + ).catch(noop); + + this.messenger.publish( + `${controllerName}:unapprovedTransactionAdded`, + instantTransactionMeta, + ); + + return { + result: this.#processApproval(instantTransactionMeta, { + actionId, + isExisting: false, + publishHook, + requireApproval, + traceContext, + }), + transactionMeta: instantTransactionMeta, + }; + } + const chainId = this.#getChainId(networkClientId); const ethQuery = this.#getEthQuery({ @@ -1570,6 +1637,12 @@ export class TransactionController extends BaseController< actionId, }: { estimatedBaseFee?: string; actionId?: string } = {}, ): Promise { + if (this.#getTransaction(transactionId)?.ready === false) { + throw new Error( + 'Cannot cancel transaction: essential async data has not resolved yet.', + ); + } + await this.#retryTransaction({ actionId, estimatedBaseFee, @@ -1614,6 +1687,12 @@ export class TransactionController extends BaseController< estimatedBaseFee, }: { actionId?: string; estimatedBaseFee?: string } = {}, ): Promise { + if (this.#getTransaction(transactionId)?.ready === false) { + throw new Error( + 'Cannot speed up transaction: essential async data has not resolved yet.', + ); + } + await this.#retryTransaction({ actionId, estimatedBaseFee, @@ -3032,6 +3111,105 @@ export class TransactionController extends BaseController< ); } + async #resolveInstantTransaction( + transactionMeta: TransactionMeta, + options: AddTransactionOptions, + ): Promise { + const { isStateOnly, requireApproval, traceContext } = options; + + const { id: transactionId, networkClientId } = transactionMeta; + + try { + const isEIP1559Compatible = + await this.#getEIP1559Compatibility(networkClientId); + + if (!transactionMeta.txParams.type) { + setEnvelopeType(transactionMeta.txParams, isEIP1559Compatible); + } + + const ethQuery = this.#getEthQuery({ networkClientId }); + const resolvedType = + transactionMeta.type ?? + (await determineTransactionType(transactionMeta.txParams, ethQuery)) + .type; + + const { updateTransaction } = await this.#afterAdd({ + transactionMeta, + }); + + if (updateTransaction) { + transactionMeta.txParamsOriginal = cloneDeep(transactionMeta.txParams); + updateTransaction(transactionMeta); + } + + await this.#updateGasProperties(transactionMeta); + + const { chainId } = transactionMeta; + validateTxParams(transactionMeta.txParams, isEIP1559Compatible, chainId); + + this.#updateTransactionInternal( + { + transactionId, + skipResimulateCheck: true, + skipValidation: true, + }, + (tx) => { + tx.txParams = { ...transactionMeta.txParams }; + tx.type = resolvedType; + tx.ready = true; + }, + ); + + const updatedMeta = this.#getTransaction(transactionId); + if (updatedMeta) { + getDelegationAddress(updatedMeta.txParams.from as Hex, ethQuery) + .then((delegationAddress) => { + this.#updateTransactionInternal( + { + transactionId, + skipResimulateCheck: true, + skipValidation: true, + }, + (tx) => { + tx.delegationAddress = delegationAddress; + }, + ); + return undefined; + }) + .catch(noop); + } + + if (requireApproval !== false && !isStateOnly) { + this.#updateSimulationData( + this.#getTransaction(transactionId) ?? transactionMeta, + { traceContext }, + ).catch((error) => { + log('Error while updating simulation data', error); + throw error; + }); + + updateFirstTimeInteraction({ + existingTransactions: this.state.transactions, + getTransaction: (id: string) => this.#getTransaction(id), + isFirstTimeInteractionEnabled: this.#isFirstTimeInteractionEnabled, + trace: this.#trace, + traceContext, + transactionMeta: + this.#getTransaction(transactionId) ?? transactionMeta, + updateTransaction: this.#updateTransactionInternal.bind(this), + }).catch((error) => { + log('Error while updating first interaction properties', error); + }); + } + } catch (error) { + log('Error resolving instant transaction', transactionId, error); + const latestMeta = this.#getTransaction(transactionId); + if (latestMeta) { + this.#failTransaction(latestMeta, error as Error); + } + } + } + #onBootCleanup(): void { this.clearUnapprovedTransactions(); this.#failIncompleteTransactions(); @@ -3248,6 +3426,16 @@ export class TransactionController extends BaseController< log('Approving transaction', transactionMeta); + if (transactionMeta.ready === false) { + this.#failTransaction( + transactionMeta, + new Error( + 'Transaction is not ready. Essential async data has not resolved yet.', + ), + ); + return ApprovalState.NotApproved; + } + try { if (!this.#sign) { this.#failTransaction( diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index fdbbf13df81..a7d9e1ce06e 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -379,6 +379,11 @@ export type TransactionMeta = { */ rawTx?: string; + /** + * Whether essential async data has been resolved for this transaction. Set to false when added with the instant option; flips to true once gas estimation, gas fee calculation, and transaction type resolution complete. Undefined for non-instant transactions (treated as ready). + */ + ready?: boolean; + /** * When the transaction is dropped, this is the replacement transaction hash. */ @@ -2175,6 +2180,9 @@ export type AddTransactionOptions = { /** Whether to disable the gas estimation buffer. */ disableGasBuffer?: boolean; + /** Whether to add the transaction to state immediately, deferring all async data loading (gas estimation, fees, type resolution) to the background. When true, the transaction is available in state instantly but `ready` will be false until essential data resolves. Only supported for internal transactions (origin undefined or ORIGIN_METAMASK). */ + instant?: boolean; + /** Address of an ERC-20 token to pay for the gas fee, if the user has insufficient native balance. */ gasFeeToken?: Hex;