diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 0548072098..cdcd263a94 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add `isExternalSign` property to `TransactionMeta` to disable nonce generation and signing ([#5604](https://github.com/MetaMask/core/pull/5604)) - Add types for `isAtomicBatchSupported` method ([#5600](https://github.com/MetaMask/core/pull/5600)) - `IsAtomicBatchSupportedRequest` - `IsAtomicBatchSupportedResult` diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index aa95dd5410..a1375ef28e 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -2452,6 +2452,45 @@ describe('TransactionController', () => { expect(publishHook).toHaveBeenCalledTimes(1); }); + it('skips signing if isExternalSign is true', async () => { + const { controller, mockTransactionApprovalRequest } = + setupController(); + + const signSpy = jest.spyOn(controller, 'sign'); + + const { result, transactionMeta } = await controller.addTransaction( + { + from: ACCOUNT_MOCK, + gas: '0x0', + gasPrice: '0x0', + to: ACCOUNT_MOCK, + value: '0x0', + }, + { + networkClientId: NETWORK_CLIENT_ID_MOCK, + }, + ); + + mockTransactionApprovalRequest.approve({ + value: { + txMeta: { + ...transactionMeta, + isExternalSign: true, + }, + }, + }); + + await result; + + expect(signSpy).not.toHaveBeenCalled(); + + expect(controller.state.transactions).toMatchObject([ + expect.objectContaining({ + status: TransactionStatus.submitted, + }), + ]); + }); + describe('fails', () => { /** * Test template to assert adding and submitting a transaction fails. diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index e7fa778a21..5f67048d72 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -1703,7 +1703,7 @@ export class TransactionController extends BaseController< } // Update same nonce local transactions as dropped and define replacedBy properties. - this.markNonceDuplicatesDropped(transactionId); + this.#markNonceDuplicatesDropped(transactionId); // Update external provided transaction with updated gas values and confirmed status. this.updateTransaction( @@ -2878,7 +2878,7 @@ export class TransactionController extends BaseController< const rawTx = await this.#trace( { name: 'Sign', parentContext: traceContext }, - () => this.signTransaction(transactionMeta), + () => this.#signTransaction(transactionMeta), ); if (!(await this.beforePublish(transactionMeta))) { @@ -2890,7 +2890,7 @@ export class TransactionController extends BaseController< return ApprovalState.SkippedViaBeforePublishHook; } - if (!rawTx) { + if (!rawTx && !transactionMeta.isExternalSign) { return ApprovalState.NotApproved; } @@ -2934,7 +2934,7 @@ export class TransactionController extends BaseController< ({ transactionHash: hash } = await publishHook( transactionMeta, - rawTx, + rawTx ?? '0x', )); if (hash === undefined) { @@ -3358,7 +3358,7 @@ export class TransactionController extends BaseController< * * @param transactionId - Used to identify original transaction. */ - private markNonceDuplicatesDropped(transactionId: string) { + #markNonceDuplicatesDropped(transactionId: string) { const transactionMeta = this.getTransaction(transactionId); if (!transactionMeta) { return; @@ -3371,6 +3371,7 @@ export class TransactionController extends BaseController< (transaction) => transaction.id !== transactionId && transaction.txParams.from === from && + nonce && transaction.txParams.nonce === nonce && transaction.chainId === chainId && transaction.type !== TransactionType.incoming, @@ -3483,10 +3484,15 @@ export class TransactionController extends BaseController< ); } - private async signTransaction( + async #signTransaction( transactionMeta: TransactionMeta, ): Promise { - const { txParams } = transactionMeta; + const { isExternalSign, txParams } = transactionMeta; + + if (isExternalSign) { + log('Skipping sign as signed externally'); + return undefined; + } log('Signing transaction', txParams); @@ -3583,10 +3589,10 @@ export class TransactionController extends BaseController< ); } - private onConfirmedTransaction(transactionMeta: TransactionMeta) { + #onConfirmedTransaction(transactionMeta: TransactionMeta) { log('Processing confirmed transaction', transactionMeta.id); - this.markNonceDuplicatesDropped(transactionMeta.id); + this.#markNonceDuplicatesDropped(transactionMeta.id); this.messagingSystem.publish( `${controllerName}:transactionConfirmed`, @@ -3730,7 +3736,7 @@ export class TransactionController extends BaseController< ) { pendingTransactionTracker.hub.on( 'transaction-confirmed', - this.onConfirmedTransaction.bind(this), + this.#onConfirmedTransaction.bind(this), ); pendingTransactionTracker.hub.on( diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts index 335006ac6c..acab9f2c03 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts @@ -526,6 +526,45 @@ describe('PendingTransactionTracker', () => { expect(listener).not.toHaveBeenCalled(); }); + + it('unless no nonce', async () => { + const listener = jest.fn(); + + const confirmedTransactionMetaMock = { + ...TRANSACTION_SUBMITTED_MOCK, + id: `${ID_MOCK}2`, + status: TransactionStatus.confirmed, + txParams: { + ...TRANSACTION_SUBMITTED_MOCK.txParams, + nonce: undefined, + }, + } as unknown as TransactionMeta; + + const submittedTransactionMetaMock = { + ...TRANSACTION_SUBMITTED_MOCK, + txParams: { + ...TRANSACTION_SUBMITTED_MOCK.txParams, + nonce: undefined, + }, + }; + + pendingTransactionTracker = new PendingTransactionTracker({ + ...options, + getTransactions: () => [ + confirmedTransactionMetaMock, + submittedTransactionMetaMock, + ], + }); + + pendingTransactionTracker.hub.addListener( + 'transaction-dropped', + listener, + ); + + await onPoll(); + + expect(listener).not.toHaveBeenCalled(); + }); }); describe('fires confirmed event', () => { diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts index 325fc9806e..dec9b29d65 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts @@ -498,6 +498,7 @@ export class PendingTransactionTracker { tx.id !== id && tx.txParams.from === txParams.from && tx.status === TransactionStatus.confirmed && + tx.txParams.nonce && tx.txParams.nonce === txParams.nonce && tx.type !== TransactionType.incoming, ); diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index f7d4c964ca..56f6265fc3 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -215,6 +215,12 @@ export type TransactionMeta = { */ id: string; + /** + * Whether the transaction is signed externally. + * No signing will be performed in the client and the `nonce` will be `undefined`. + */ + isExternalSign?: boolean; + /** * Whether the transaction is a transfer. */ diff --git a/packages/transaction-controller/src/utils/nonce.test.ts b/packages/transaction-controller/src/utils/nonce.test.ts index d4a2dc4405..c3323c61cd 100644 --- a/packages/transaction-controller/src/utils/nonce.test.ts +++ b/packages/transaction-controller/src/utils/nonce.test.ts @@ -78,6 +78,21 @@ describe('nonce', () => { expect(releaseLock).toHaveBeenCalledTimes(1); }); + + it('returns undefined if transaction is signed externally', async () => { + const transactionMeta = { + ...TRANSACTION_META_MOCK, + isExternalSign: true, + }; + + const [nonce, releaseLock] = await getNextNonce( + transactionMeta, + jest.fn(), + ); + + expect(nonce).toBeUndefined(); + expect(releaseLock).toBeUndefined(); + }); }); describe('getAndFormatTransactionsForNonceTracker', () => { diff --git a/packages/transaction-controller/src/utils/nonce.ts b/packages/transaction-controller/src/utils/nonce.ts index bda28bf22f..318b697514 100644 --- a/packages/transaction-controller/src/utils/nonce.ts +++ b/packages/transaction-controller/src/utils/nonce.ts @@ -19,12 +19,17 @@ const log = createModuleLogger(projectLogger, 'nonce'); export async function getNextNonce( txMeta: TransactionMeta, getNonceLock: (address: string) => Promise, -): Promise<[string, (() => void) | undefined]> { +): Promise<[string | undefined, (() => void) | undefined]> { const { customNonceValue, + isExternalSign, txParams: { from, nonce: existingNonce }, } = txMeta; + if (isExternalSign) { + return [undefined, undefined]; + } + const customNonce = customNonceValue ? toHex(customNonceValue) : undefined; if (customNonce) {