diff --git a/src/SmartTransactionsController.test.ts b/src/SmartTransactionsController.test.ts index bcb2b7c9..53025fa0 100644 --- a/src/SmartTransactionsController.test.ts +++ b/src/SmartTransactionsController.test.ts @@ -723,6 +723,183 @@ describe('SmartTransactionsController', () => { }); }); + it('should acquire nonce for Swap transactions only', async () => { + // Create a mock for getNonceLock + const mockGetNonceLock = jest.fn().mockResolvedValue({ + nextNonce: 'nextNonce', + nonceDetails: { test: 'details' }, + releaseLock: jest.fn(), + }); + + await withController( + { + options: { + getNonceLock: mockGetNonceLock, + }, + }, + async ({ controller }) => { + const signedTransaction = createSignedTransaction(); + const submitTransactionsApiResponse = + createSubmitTransactionsApiResponse(); + + // First API mock for the case without nonce + nock(API_BASE_URL) + .post( + `/networks/${ethereumChainIdDec}/submitTransactions?stxControllerVersion=${packageJson.version}`, + ) + .reply(200, submitTransactionsApiResponse); + + // Second API mock for the case with nonce + nock(API_BASE_URL) + .post( + `/networks/${ethereumChainIdDec}/submitTransactions?stxControllerVersion=${packageJson.version}`, + ) + .reply(200, submitTransactionsApiResponse); + + // Case 1: Swap transaction without nonce (should call getNonceLock) + const txParamsWithoutNonce = { + ...createTxParams(), + nonce: undefined, // Explicitly undefined nonce + }; + + await controller.submitSignedTransactions({ + signedTransactions: [signedTransaction], + txParams: txParamsWithoutNonce, + // No transactionMeta means type defaults to 'swap' + }); + + // Verify getNonceLock was called for the Swap + expect(mockGetNonceLock).toHaveBeenCalledTimes(1); + expect(mockGetNonceLock).toHaveBeenCalledWith( + txParamsWithoutNonce.from, + NetworkType.mainnet, + ); + + // Reset the mock + mockGetNonceLock.mockClear(); + + // Case 2: Transaction with nonce already set (should NOT call getNonceLock) + const txParamsWithNonce = createTxParams(); // This has nonce: '0' + + await controller.submitSignedTransactions({ + signedTransactions: [signedTransaction], + txParams: txParamsWithNonce, + }); + + // Verify getNonceLock was NOT called for transaction with nonce + expect(mockGetNonceLock).not.toHaveBeenCalled(); + }, + ); + }); + + it('should properly set nonce on txParams and mark transaction as swap type', async () => { + // Mock with a specific nextNonce value we can verify + const mockGetNonceLock = jest.fn().mockResolvedValue({ + nextNonce: 42, + nonceDetails: { test: 'nonce details' }, + releaseLock: jest.fn(), + }); + + await withController( + { + options: { + getNonceLock: mockGetNonceLock, + }, + }, + async ({ controller }) => { + const signedTransaction = createSignedTransaction(); + const submitTransactionsApiResponse = + createSubmitTransactionsApiResponse(); + nock(API_BASE_URL) + .post( + `/networks/${ethereumChainIdDec}/submitTransactions?stxControllerVersion=${packageJson.version}`, + ) + .reply(200, submitTransactionsApiResponse); + + // Create txParams without nonce + const txParamsWithoutNonce = { + ...createTxParams(), + nonce: undefined, + from: addressFrom, + }; + + await controller.submitSignedTransactions({ + signedTransactions: [signedTransaction], + txParams: txParamsWithoutNonce, + // No transactionMeta provided, should default to 'swap' type + }); + + // Get the created smart transaction + const createdSmartTransaction = + controller.state.smartTransactionsState.smartTransactions[ + ChainId.mainnet + ][0]; + + // Verify nonce was set correctly on the txParams in the created transaction + expect(createdSmartTransaction.txParams.nonce).toBe('0x42'); // 42 as a hex string + + // Verify transaction type is set to 'swap' by default + expect(createdSmartTransaction.type).toBe('swap'); + + // Verify nonceDetails were passed correctly + expect(createdSmartTransaction.nonceDetails).toStrictEqual({ + test: 'nonce details', + }); + }, + ); + }); + + it('should handle errors when acquiring nonce lock', async () => { + // Mock getNonceLock to reject with an error + const mockError = new Error('Failed to acquire nonce'); + const mockGetNonceLock = jest.fn().mockRejectedValue(mockError); + + // Spy on console.error to verify it's called + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); + + await withController( + { + options: { + getNonceLock: mockGetNonceLock, + }, + }, + async ({ controller }) => { + const signedTransaction = createSignedTransaction(); + const submitTransactionsApiResponse = + createSubmitTransactionsApiResponse(); + nock(API_BASE_URL) + .post( + `/networks/${ethereumChainIdDec}/submitTransactions?stxControllerVersion=${packageJson.version}`, + ) + .reply(200, submitTransactionsApiResponse); + + // Create txParams without nonce + const txParamsWithoutNonce = { + ...createTxParams(), + nonce: undefined, + from: addressFrom, + }; + + // Attempt to submit a transaction that will fail when acquiring nonce + await expect( + controller.submitSignedTransactions({ + signedTransactions: [signedTransaction], + txParams: txParamsWithoutNonce, + }), + ).rejects.toThrow('Failed to acquire nonce'); + + // Verify error was logged + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to acquire nonce lock:', + mockError, + ); + + // Cleanup spy + consoleErrorSpy.mockRestore(); + }, + ); + }); + it('submits a batch of signed transactions', async () => { await withController(async ({ controller }) => { const signedTransaction1 = createSignedTransaction(); @@ -2090,6 +2267,126 @@ describe('SmartTransactionsController', () => { ); }); }); + + describe('createOrUpdateSmartTransaction', () => { + beforeEach(() => { + jest + .spyOn(SmartTransactionsController.prototype, 'checkPoll') + .mockImplementation(() => ({})); + }); + + it('adds metaMetricsProps to new smart transactions', async () => { + const { smartTransactionsState } = + getDefaultSmartTransactionsControllerState(); + const newSmartTransaction = { + uuid: 'new-uuid-test', + status: SmartTransactionStatuses.PENDING, + txParams: { + from: addressFrom, + }, + }; + + await withController( + { + options: { + state: { + smartTransactionsState: { + ...smartTransactionsState, + smartTransactions: { + [ChainId.mainnet]: [], + }, + }, + }, + getMetaMetricsProps: jest.fn().mockResolvedValue({ + accountHardwareType: 'Test Hardware', + accountType: 'test-account', + deviceModel: 'test-model', + }), + }, + }, + async ({ controller }) => { + controller.updateSmartTransaction( + newSmartTransaction as SmartTransaction, + ); + + // Allow async operations to complete + await flushPromises(); + + // Verify MetaMetricsProps were added + const updatedTransaction = + controller.state.smartTransactionsState.smartTransactions[ + ChainId.mainnet + ][0]; + expect(updatedTransaction.accountHardwareType).toBe('Test Hardware'); + expect(updatedTransaction.accountType).toBe('test-account'); + expect(updatedTransaction.deviceModel).toBe('test-model'); + }, + ); + }); + + it('continues without metaMetricsProps if adding them fails', async () => { + const { smartTransactionsState } = + getDefaultSmartTransactionsControllerState(); + const newSmartTransaction = { + uuid: 'new-uuid-test', + status: SmartTransactionStatuses.PENDING, + txParams: { + from: addressFrom, + }, + }; + + // Mock console.error to verify it's called + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); + + await withController( + { + options: { + state: { + smartTransactionsState: { + ...smartTransactionsState, + smartTransactions: { + [ChainId.mainnet]: [], + }, + }, + }, + // Mock getting MetaMetricsProps to fail + getMetaMetricsProps: jest + .fn() + .mockRejectedValue(new Error('Test metrics error')), + }, + }, + async ({ controller }) => { + controller.updateSmartTransaction( + newSmartTransaction as SmartTransaction, + ); + + // Allow async operations to complete + await flushPromises(); + + // Verify transaction was still added even without metrics props + const updatedTransaction = + controller.state.smartTransactionsState.smartTransactions[ + ChainId.mainnet + ][0]; + expect(updatedTransaction.uuid).toBe('new-uuid-test'); + + // These should be undefined since getting metrics props failed + expect(updatedTransaction.accountHardwareType).toBeUndefined(); + expect(updatedTransaction.accountType).toBeUndefined(); + expect(updatedTransaction.deviceModel).toBeUndefined(); + + // Verify the error was logged + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to add metrics props to smart transaction:', + expect.any(Error), + ); + + // Clean up the spy + consoleErrorSpy.mockRestore(); + }, + ); + }); + }); }); type WithControllerCallback = ({ diff --git a/src/SmartTransactionsController.ts b/src/SmartTransactionsController.ts index ac3aa45f..3615978e 100644 --- a/src/SmartTransactionsController.ts +++ b/src/SmartTransactionsController.ts @@ -513,12 +513,20 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo smartTransaction.uuid, chainId, ); - if (this.#ethQuery === undefined) { + if (ethQuery === undefined) { throw new Error(ETH_QUERY_ERROR_MSG); } if (isNewSmartTransaction) { - await this.#addMetaMetricsPropsToNewSmartTransaction(smartTransaction); + try { + await this.#addMetaMetricsPropsToNewSmartTransaction(smartTransaction); + } catch (error) { + console.error( + 'Failed to add metrics props to smart transaction:', + error, + ); + // Continue without metrics props + } } this.trackStxStatusChange( @@ -955,7 +963,7 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo preTxBalance = new BigNumber(preTxBalanceBN).toString(16); } } catch (error) { - console.error('provider error', error); + console.error('ethQuery.getBalance error:', error); } const requiresNonce = txParams && !txParams.nonce; @@ -963,20 +971,26 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo let nonceLock; let nonceDetails = {}; + // This should only happen for Swaps. Non-swaps transactions should already have a nonce if (requiresNonce) { - nonceLock = await this.#getNonceLock( - txParams.from, - selectedNetworkClientId, - ); - nonce = hexlify(nonceLock.nextNonce); - nonceDetails = nonceLock.nonceDetails; - txParams.nonce ??= nonce; + try { + nonceLock = await this.#getNonceLock( + txParams.from, + selectedNetworkClientId, + ); + nonce = hexlify(nonceLock.nextNonce); + nonceDetails = nonceLock.nonceDetails; + txParams.nonce ??= nonce; + } catch (error) { + console.error('Failed to acquire nonce lock:', error); + throw error; + } } const txHashes = signedTransactions.map((tx) => getTxHash(tx)); const submitTransactionResponse = { ...data, - txHash: txHashes[0], // For backward compatibility + txHash: txHashes[txHashes.length - 1], // For backward compatibility - use the last tx hash txHashes, }; @@ -999,8 +1013,13 @@ export default class SmartTransactionsController extends StaticIntervalPollingCo }, { chainId, ethQuery }, ); + } catch (error) { + console.error('Failed to create a smart transaction:', error); + throw error; } finally { - nonceLock?.releaseLock(); + if (nonceLock) { + nonceLock.releaseLock(); + } } return submitTransactionResponse;