diff --git a/packages/bridge-status-controller/CHANGELOG.md b/packages/bridge-status-controller/CHANGELOG.md index 11f4fec7421..559bff784c8 100644 --- a/packages/bridge-status-controller/CHANGELOG.md +++ b/packages/bridge-status-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Fix transaction failure tracking for pre-submission failures by using `actionId` as a temporary history key ([#7696](https://github.com/MetaMask/core/pull/7696)) + ## [64.4.4] ### Changed diff --git a/packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap b/packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap index a3a47e29fa4..f8325db8bb3 100644 --- a/packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap +++ b/packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap @@ -4,6 +4,7 @@ exports[`BridgeStatusController constructor rehydrates the tx history state 1`] Object { "bridgeTxMetaId1": Object { "account": "0xaccount1", + "actionId": undefined, "approvalTxId": undefined, "attempts": undefined, "batchId": undefined, @@ -208,6 +209,7 @@ exports[`BridgeStatusController startPollingForBridgeTxStatus sets the inital tx Object { "bridgeTxMetaId1": Object { "account": "0xaccount1", + "actionId": undefined, "approvalTxId": undefined, "batchId": undefined, "estimatedProcessingTimeInSeconds": 15, @@ -410,6 +412,7 @@ Object { exports[`BridgeStatusController submitTx: EVM bridge should call handleMobileHardwareWalletDelay for hardware wallet on mobile 2`] = ` Object { "account": "0xaccount1", + "actionId": "1234567890.456", "approvalTxId": "test-approval-tx-id", "batchId": undefined, "estimatedProcessingTimeInSeconds": 15, @@ -646,6 +649,7 @@ Object { exports[`BridgeStatusController submitTx: EVM bridge should delay after submitting base approval 2`] = ` Object { "account": "0xaccount1", + "actionId": "1234567890.456", "approvalTxId": "test-approval-tx-id", "batchId": undefined, "estimatedProcessingTimeInSeconds": 15, @@ -882,6 +886,7 @@ Object { exports[`BridgeStatusController submitTx: EVM bridge should delay after submitting linea approval 2`] = ` Object { "account": "0xaccount1", + "actionId": "1234567890.456", "approvalTxId": "test-approval-tx-id", "batchId": undefined, "estimatedProcessingTimeInSeconds": 15, @@ -1119,6 +1124,7 @@ Object { exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and include quotesReceivedContext 2`] = ` Object { "account": "0xaccount1", + "actionId": undefined, "approvalTxId": undefined, "batchId": "batchId1", "estimatedProcessingTimeInSeconds": 15, @@ -1382,6 +1388,7 @@ Object { exports[`BridgeStatusController submitTx: EVM bridge should not call handleMobileHardwareWalletDelay on extension 2`] = ` Object { "account": "0xaccount1", + "actionId": "1234567890.456", "approvalTxId": "test-approval-tx-id", "batchId": undefined, "estimatedProcessingTimeInSeconds": 15, @@ -1618,6 +1625,7 @@ Object { exports[`BridgeStatusController submitTx: EVM bridge should not call handleMobileHardwareWalletDelay with true for non-hardware wallet on mobile 2`] = ` Object { "account": "0xaccount1", + "actionId": "1234567890.456", "approvalTxId": "test-approval-tx-id", "batchId": undefined, "estimatedProcessingTimeInSeconds": 15, @@ -1854,6 +1862,7 @@ Object { exports[`BridgeStatusController submitTx: EVM bridge should reset USDT allowance 2`] = ` Object { "account": "0xaccount1", + "actionId": "1234567890.456", "approvalTxId": "test-approval-tx-id", "batchId": undefined, "estimatedProcessingTimeInSeconds": 15, @@ -2194,6 +2203,7 @@ Object { exports[`BridgeStatusController submitTx: EVM bridge should successfully submit an EVM bridge transaction with approval 2`] = ` Object { "account": "0xaccount1", + "actionId": "1234567890.456", "approvalTxId": "test-approval-tx-id", "batchId": undefined, "estimatedProcessingTimeInSeconds": 15, @@ -2450,6 +2460,7 @@ Object { exports[`BridgeStatusController submitTx: EVM bridge should successfully submit an EVM bridge transaction with no approval 2`] = ` Object { "account": "0xaccount1", + "actionId": "1234567890.456", "approvalTxId": undefined, "batchId": undefined, "estimatedProcessingTimeInSeconds": 15, @@ -2857,6 +2868,7 @@ Object { exports[`BridgeStatusController submitTx: EVM swap should handle smart transactions 2`] = ` Object { "account": "0xaccount1", + "actionId": undefined, "approvalTxId": undefined, "batchId": "batchId1", "estimatedProcessingTimeInSeconds": 0, @@ -3210,6 +3222,7 @@ Object { exports[`BridgeStatusController submitTx: EVM swap should successfully submit an EVM swap transaction with no approval 2`] = ` Object { "account": "0xaccount1", + "actionId": "1234567890.456", "approvalTxId": undefined, "batchId": undefined, "estimatedProcessingTimeInSeconds": 0, @@ -3423,6 +3436,7 @@ Object { exports[`BridgeStatusController submitTx: EVM swap should use quote txFee when gasIncluded is true and STX is off (Max native token swap) 2`] = ` Object { "account": "0xaccount1", + "actionId": "1234567890.456", "approvalTxId": undefined, "batchId": undefined, "estimatedProcessingTimeInSeconds": 0, @@ -3722,6 +3736,7 @@ Object { exports[`BridgeStatusController submitTx: Solana bridge should successfully submit a transaction 4`] = ` Object { "account": "0x123...", + "actionId": undefined, "approvalTxId": undefined, "batchId": undefined, "estimatedProcessingTimeInSeconds": 300, @@ -4057,6 +4072,7 @@ Object { exports[`BridgeStatusController submitTx: Solana swap should successfully submit a transaction 3`] = ` Object { "account": "0x123...", + "actionId": undefined, "approvalTxId": undefined, "batchId": undefined, "estimatedProcessingTimeInSeconds": 300, @@ -4433,6 +4449,7 @@ Object { exports[`BridgeStatusController submitTx: Tron swap with approval should successfully submit a Tron bridge with approval transaction 4`] = ` Object { "account": "TRX123...", + "actionId": undefined, "approvalTxId": "approval-signature", "batchId": undefined, "estimatedProcessingTimeInSeconds": 30, @@ -4652,6 +4669,7 @@ Object { exports[`BridgeStatusController submitTx: Tron swap with approval should successfully submit a Tron swap with approval transaction 3`] = ` Object { "account": "TRX123...", + "actionId": undefined, "approvalTxId": "approval-signature", "batchId": undefined, "estimatedProcessingTimeInSeconds": 30, @@ -4901,6 +4919,46 @@ Array [ ] `; +exports[`BridgeStatusController subscription handlers TransactionController:transactionFailed should find history by actionId when txMeta.id not in history (pre-submission failure) 1`] = ` +Array [ + "BridgeController:trackUnifiedSwapBridgeEvent", + "Unified SwapBridge Failed", + Object { + "action_type": "swapbridge-v1", + "actual_time_minutes": 0, + "allowance_reset_transaction": undefined, + "approval_transaction": undefined, + "chain_id_destination": "eip155:10", + "chain_id_source": "eip155:42161", + "custom_slippage": true, + "destination_transaction": "FAILED", + "error_message": "", + "gas_included": false, + "gas_included_7702": false, + "is_hardware_wallet": false, + "price_impact": 0, + "provider": "lifi_across", + "quote_vs_execution_ratio": 0, + "quoted_time_minutes": 0.25, + "quoted_vs_used_gas_ratio": 0, + "security_warnings": Array [], + "slippage_limit": 0, + "source_transaction": "COMPLETE", + "stx_enabled": false, + "swap_type": "crosschain", + "token_address_destination": "eip155:10/slip44:60", + "token_address_source": "eip155:42161/slip44:60", + "token_symbol_destination": "ETH", + "token_symbol_source": "ETH", + "usd_actual_gas": 0, + "usd_actual_return": 0, + "usd_amount_source": 0, + "usd_quoted_gas": 2.5778, + "usd_quoted_return": 0, + }, +] +`; + exports[`BridgeStatusController subscription handlers TransactionController:transactionFailed should not track failed event for approved status 1`] = `Array []`; exports[`BridgeStatusController subscription handlers TransactionController:transactionFailed should not track failed event for other transaction types 1`] = `Array []`; diff --git a/packages/bridge-status-controller/src/bridge-status-controller.test.ts b/packages/bridge-status-controller/src/bridge-status-controller.test.ts index d9695b5dfe9..5af1c07f817 100644 --- a/packages/bridge-status-controller/src/bridge-status-controller.test.ts +++ b/packages/bridge-status-controller/src/bridge-status-controller.test.ts @@ -342,12 +342,14 @@ const getMockStartPollingForBridgeTxStatusArgs = ({ const MockTxHistory = { getInitNoSrcTxHash: ({ txMetaId = 'bridgeTxMetaId1', + actionId = undefined, account = '0xaccount1', srcChainId = 42161, destChainId = 10, } = {}): Record => ({ [txMetaId]: { txMetaId, + actionId, originalTransactionId: txMetaId, quote: getMockQuote({ srcChainId, destChainId }), startTime: 1729964825189, @@ -366,12 +368,14 @@ const MockTxHistory = { }), getInit: ({ txMetaId = 'bridgeTxMetaId1', + actionId = undefined, account = '0xaccount1', srcChainId = 42161, destChainId = 10, } = {}): Record => ({ [txMetaId]: { txMetaId, + actionId, originalTransactionId: txMetaId, quote: getMockQuote({ srcChainId, destChainId }), startTime: 1729964825189, @@ -390,6 +394,7 @@ const MockTxHistory = { getPending: ({ txMetaId = 'bridgeTxMetaId1', batchId = undefined, + actionId = undefined, approvalTxId = undefined, srcTxHash = '0xsrcTxHash1', account = '0xaccount1', @@ -399,6 +404,7 @@ const MockTxHistory = { } = {}): Record => ({ [txMetaId]: { txMetaId, + actionId, originalTransactionId: txMetaId, batchId, quote: getMockQuote({ srcChainId, destChainId }), @@ -429,6 +435,7 @@ const MockTxHistory = { }), getUnknown: ({ txMetaId = 'bridgeTxMetaId2', + actionId = undefined, srcTxHash = '0xsrcTxHash2', account = '0xaccount1', srcChainId = 42161, @@ -436,6 +443,7 @@ const MockTxHistory = { } = {}): Record => ({ [txMetaId]: { txMetaId, + actionId, originalTransactionId: txMetaId, quote: getMockQuote({ srcChainId, destChainId }), startTime: 1729964825189, @@ -464,6 +472,7 @@ const MockTxHistory = { }), getPendingSwap: ({ txMetaId = 'swapTxMetaId1', + actionId = undefined, srcTxHash = '0xsrcTxHash1', account = '0xaccount1', srcChainId = 42161, @@ -472,6 +481,7 @@ const MockTxHistory = { } = {}): Record => ({ [txMetaId]: { txMetaId, + actionId, originalTransactionId: txMetaId, quote: getMockQuote({ srcChainId, destChainId }), startTime: 1729964825189, @@ -499,6 +509,7 @@ const MockTxHistory = { }), getComplete: ({ txMetaId = 'bridgeTxMetaId1', + actionId = undefined, batchId = undefined, srcTxHash = '0xsrcTxHash1', account = '0xaccount1', @@ -507,6 +518,7 @@ const MockTxHistory = { } = {}): Record => ({ [txMetaId]: { txMetaId, + actionId, originalTransactionId: txMetaId, batchId, featureId: undefined, @@ -906,6 +918,54 @@ describe('BridgeStatusController', () => { jest.clearAllMocks(); }); + it('throws error when bridgeTxMeta.id is not provided', () => { + const bridgeStatusController = new BridgeStatusController({ + messenger: getMessengerMock(), + clientId: BridgeClientId.EXTENSION, + fetchFn: jest.fn(), + addTransactionFn: jest.fn(), + addTransactionBatchFn: jest.fn(), + updateTransactionFn: jest.fn(), + estimateGasFeeFn: jest.fn(), + }); + + const argsWithoutId = getMockStartPollingForBridgeTxStatusArgs(); + // Remove the id from bridgeTxMeta + argsWithoutId.bridgeTxMeta = {} as never; + + expect(() => { + bridgeStatusController.startPollingForBridgeTxStatus(argsWithoutId); + }).toThrow( + 'Cannot start polling: bridgeTxMeta.id is required for polling', + ); + + bridgeStatusController.stopAllPolling(); + }); + + it('throws error when bridgeTxMeta is undefined', () => { + const bridgeStatusController = new BridgeStatusController({ + messenger: getMessengerMock(), + clientId: BridgeClientId.EXTENSION, + fetchFn: jest.fn(), + addTransactionFn: jest.fn(), + addTransactionBatchFn: jest.fn(), + updateTransactionFn: jest.fn(), + estimateGasFeeFn: jest.fn(), + }); + + const argsWithoutMeta = getMockStartPollingForBridgeTxStatusArgs(); + // Remove bridgeTxMeta entirely + argsWithoutMeta.bridgeTxMeta = undefined as never; + + expect(() => { + bridgeStatusController.startPollingForBridgeTxStatus(argsWithoutMeta); + }).toThrow( + 'Cannot start polling: bridgeTxMeta.id is required for polling', + ); + + bridgeStatusController.stopAllPolling(); + }); + it('sets the inital tx history state', async () => { // Setup const bridgeStatusController = new BridgeStatusController({ @@ -3039,6 +3099,131 @@ describe('BridgeStatusController', () => { expect(mockMessengerCall.mock.calls).toMatchSnapshot(); expect(mockTraceFn.mock.calls).toMatchSnapshot(); }); + + describe('actionId tracking and rekeying', () => { + it('should add pre-submission history keyed by actionId and rekey to txMeta.id after success', async () => { + // Mock generateActionId to return a predictable value + const mockActionId = '1234567890.456'; + jest + .spyOn(transactionUtils, 'generateActionId') + .mockReturnValue(mockActionId); + + setupEventTrackingMocks(mockMessengerCall); + // No approval for this test - direct to bridge tx + const { approval, ...quoteWithoutApproval } = mockEvmQuoteResponse; + setupBridgeMocks(mockMessengerCall); + + const { controller, startPollingForBridgeTxStatusSpy } = + getController(mockMessengerCall); + + const result = await controller.submitTx( + (quoteWithoutApproval.trade as TxData).from, + quoteWithoutApproval, + false, // STX disabled - uses non-batch path + ); + controller.stopAllPolling(); + + // Verify the final history is keyed by txMeta.id (not actionId) + expect(controller.state.txHistory[result.id]).toBeDefined(); + expect(controller.state.txHistory[result.id].txMetaId).toBe(result.id); + expect(controller.state.txHistory[result.id].actionId).toBe( + mockActionId, + ); + + // Verify the actionId key no longer exists (was rekeyed) + expect(controller.state.txHistory[mockActionId]).toBeUndefined(); + + // Verify srcTxHash was updated during rekey + expect( + controller.state.txHistory[result.id].status.srcChain.txHash, + ).toBe(result.hash); + + expect(startPollingForBridgeTxStatusSpy).toHaveBeenCalledTimes(0); + }); + + it('should preserve pre-submission history for tracking when trade tx submission fails', async () => { + const mockActionId = '9876543210.789'; + jest + .spyOn(transactionUtils, 'generateActionId') + .mockReturnValue(mockActionId); + + setupEventTrackingMocks(mockMessengerCall); + const { approval, ...quoteWithoutApproval } = mockEvmQuoteResponse; + + // Setup for trade tx (no approval) + mockMessengerCall.mockReturnValueOnce(mockSelectedAccount); + mockMessengerCall.mockReturnValueOnce('arbitrum-client-id'); + mockMessengerCall.mockReturnValueOnce({ + gasFeeEstimates: { estimatedBaseFee: '0x1234' }, + }); + estimateGasFeeFn.mockResolvedValueOnce({ + estimates: { + high: { + suggestedMaxFeePerGas: '0x1234', + suggestedMaxPriorityFeePerGas: '0x5678', + }, + }, + }); + + // Trade tx fails during submission + addTransactionFn.mockRejectedValueOnce( + new Error('Trade tx submission failed'), + ); + + const { controller, startPollingForBridgeTxStatusSpy } = + getController(mockMessengerCall); + + await expect( + controller.submitTx( + (quoteWithoutApproval.trade as TxData).from, + quoteWithoutApproval, + false, + ), + ).rejects.toThrow('Trade tx submission failed'); + + // Verify: Pre-submission history should still exist keyed by actionId + // This allows failed event tracking to find the quote data + expect(controller.state.txHistory[mockActionId]).toBeDefined(); + expect(controller.state.txHistory[mockActionId].actionId).toBe( + mockActionId, + ); + expect( + controller.state.txHistory[mockActionId].txMetaId, + ).toBeUndefined(); + expect( + controller.state.txHistory[mockActionId].status.srcChain.txHash, + ).toBe(''); // Empty since tx was never submitted + + expect(startPollingForBridgeTxStatusSpy).toHaveBeenCalledTimes(0); + }); + + it('should use provided actionId from addTransactionFn result', async () => { + const mockActionId = '1111111111.222'; + jest + .spyOn(transactionUtils, 'generateActionId') + .mockReturnValue(mockActionId); + + setupEventTrackingMocks(mockMessengerCall); + setupApprovalMocks(mockMessengerCall); + setupBridgeMocks(mockMessengerCall); + + const { controller, startPollingForBridgeTxStatusSpy } = + getController(mockMessengerCall); + + const result = await controller.submitTx( + (mockEvmQuoteResponse.trade as TxData).from, + mockEvmQuoteResponse, + false, // STX disabled + ); + controller.stopAllPolling(); + + // Verify actionId is stored in the history item + expect(controller.state.txHistory[result.id].actionId).toBe( + mockActionId, + ); + expect(startPollingForBridgeTxStatusSpy).toHaveBeenCalledTimes(0); + }); + }); }); describe('submitTx: EVM swap', () => { @@ -3957,6 +4142,10 @@ describe('BridgeStatusController', () => { mockFetchFn = jest .fn() .mockResolvedValueOnce(MockStatusResponse.getPending()); + + // Create base history item for actionId-keyed entries + const baseHistoryItem = MockTxHistory.getPending().bridgeTxMetaId1; + bridgeStatusController = new BridgeStatusController({ messenger: mockBridgeStatusMessenger, clientId: BridgeClientId.EXTENSION, @@ -3982,6 +4171,22 @@ describe('BridgeStatusController', () => { srcTxHash: '0xperpsSrcTxHash1', featureId: FeatureId.PERPS as never, }), + // ActionId-keyed entries for pre-submission failure tests + 'pre-submission-action-id': { + ...baseHistoryItem, + actionId: 'pre-submission-action-id', + txMetaId: undefined, + } as BridgeHistoryItem, + 'action-id-for-tracking': { + ...baseHistoryItem, + actionId: 'action-id-for-tracking', + txMetaId: undefined, + } as BridgeHistoryItem, + 'action-id-for-rejection': { + ...baseHistoryItem, + actionId: 'action-id-for-rejection', + txMetaId: undefined, + } as BridgeHistoryItem, }, }, }); @@ -4176,6 +4381,91 @@ describe('BridgeStatusController', () => { expect(messengerCallSpy.mock.calls).toMatchSnapshot(); }); + + it('should find history by actionId when txMeta.id not in history (pre-submission failure)', () => { + // The history entry keyed by actionId is set up in beforeEach + const actionId = 'pre-submission-action-id'; + const unknownTxMetaId = 'unknown-tx-meta-id'; + + const messengerCallSpy = jest.spyOn(mockBridgeStatusMessenger, 'call'); + + // Publish failure with an unknown txMeta.id but with matching actionId + mockMessenger.publish('TransactionController:transactionFailed', { + error: 'tx-error', + transactionMeta: { + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.failed, + id: unknownTxMetaId, + actionId, // ActionId matches the history entry + }, + }); + + // Verify: History entry keyed by actionId should be marked as failed + expect( + bridgeStatusController.state.txHistory[actionId].status.status, + ).toBe(StatusTypes.FAILED); + expect(messengerCallSpy.mock.lastCall).toMatchSnapshot(); + }); + + it('should track failed event using actionId lookup when id not found', () => { + // The history entry keyed by actionId is set up in beforeEach + const actionId = 'action-id-for-tracking'; + + const messengerCallSpy = jest.spyOn(mockBridgeStatusMessenger, 'call'); + + mockMessenger.publish('TransactionController:transactionFailed', { + error: 'tx-error', + transactionMeta: { + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.failed, + id: 'non-existent-tx-id', + actionId, + }, + }); + + // The Failed event should be tracked with the history data from actionId lookup + expect(messengerCallSpy).toHaveBeenCalled(); + expect( + bridgeStatusController.state.txHistory[actionId].status.status, + ).toBe(StatusTypes.FAILED); + }); + + it('should not track failed event when transaction is rejected', () => { + // The history entry keyed by actionId is set up in beforeEach + const actionId = 'action-id-for-rejection'; + + const messengerCallSpy = jest.spyOn(mockBridgeStatusMessenger, 'call'); + + mockMessenger.publish('TransactionController:transactionFailed', { + error: 'User rejected', + transactionMeta: { + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.rejected, + id: 'rejected-tx-id', + actionId, + }, + }); + + // Status should still be marked as failed + expect( + bridgeStatusController.state.txHistory[actionId].status.status, + ).toBe(StatusTypes.FAILED); + // But Failed event should NOT be tracked for rejected status + // (check that call was not made for tracking - only for marking failed) + expect(messengerCallSpy).not.toHaveBeenCalled(); + }); }); describe('TransactionController:transactionConfirmed', () => { diff --git a/packages/bridge-status-controller/src/bridge-status-controller.ts b/packages/bridge-status-controller/src/bridge-status-controller.ts index 555dde64118..7383d6d963e 100644 --- a/packages/bridge-status-controller/src/bridge-status-controller.ts +++ b/packages/bridge-status-controller/src/bridge-status-controller.ts @@ -78,6 +78,8 @@ import { findAndUpdateTransactionsInBatch, getAddTransactionBatchParams, getClientRequest, + getHistoryKey, + getIntentFromQuote, getStatusRequestParams, handleApprovalDelay, handleMobileHardwareWalletDelay, @@ -213,7 +215,7 @@ export class BridgeStatusController extends StaticIntervalPollingController { - const { type, status, id } = transactionMeta; + const { type, status, id: txMetaId, actionId } = transactionMeta; if ( type && @@ -233,9 +235,17 @@ export class BridgeStatusController extends StaticIntervalPollingController { - const { type, id, chainId } = transactionMeta; + const { type, id: txMetaId, chainId } = transactionMeta; if (type === TransactionType.swap) { this.#trackUnifiedSwapBridgeEvent( UnifiedSwapBridgeEventName.Completed, - id, + txMetaId, ); } if (type === TransactionType.bridge && !isNonEvmChainId(chainId)) { - this.#startPollingForTxId(id); + this.#startPollingForTxId(txMetaId); } }, ); @@ -266,17 +276,31 @@ export class BridgeStatusController extends StaticIntervalPollingController { - const txHistoryKey = this.state.txHistory[id] - ? id - : Object.keys(this.state.txHistory).find( - (key) => this.state.txHistory[key].approvalTxId === id, - ); + readonly #markTxAsFailed = ({ + id: txMetaId, + actionId, + }: TransactionMeta): void => { + // Look up by txMetaId first + let txHistoryKey: string | undefined = this.state.txHistory[txMetaId] + ? txMetaId + : undefined; + + // If not found by txMetaId, try looking up by actionId (for pre-submission failures) + if (!txHistoryKey && actionId && this.state.txHistory[actionId]) { + txHistoryKey = actionId; + } + + // If still not found, try looking up by approvalTxId + txHistoryKey ??= Object.keys(this.state.txHistory).find( + (key) => this.state.txHistory[key].approvalTxId === txMetaId, + ); + if (!txHistoryKey) { return; } + const key = txHistoryKey; this.update((statusState) => { - statusState.txHistory[txHistoryKey].status.status = StatusTypes.FAILED; + statusState.txHistory[key].status.status = StatusTypes.FAILED; }); }; @@ -406,6 +430,13 @@ export class BridgeStatusController extends StaticIntervalPollingController + Boolean(historyItem.txMetaId), + ) .filter((historyItem) => { // Check if we are already polling this tx, if so, skip restarting polling for that const pollingToken = @@ -438,6 +469,7 @@ export class BridgeStatusController extends StaticIntervalPollingController { const { bridgeTxMeta, @@ -452,15 +484,20 @@ export class BridgeStatusController extends StaticIntervalPollingController { - // Use the txMeta.id as the key so we can reference the txMeta in TransactionController - state.txHistory[bridgeTxMeta.id] = txHistoryItem; + // Use actionId as key for pre-submission, or txMeta.id for post-submission + state.txHistory[historyKey] = txHistoryItem; + }); + }; + + /** + * Rekeys a history item from actionId to txMeta.id after successful submission. + * Also updates txMetaId and srcTxHash which weren't available pre-submission. + * + * @param actionId - The actionId used as the temporary key for the history item + * @param txMeta - The transaction meta from the successful submission + * @param txMeta.id - The transaction meta id to use as the new key + * @param txMeta.hash - The transaction hash to set on the history item + */ + readonly #rekeyHistoryItem = ( + actionId: string, + txMeta: { id: string; hash?: string }, + ): void => { + const historyItem = this.state.txHistory[actionId]; + if (!historyItem) { + return; + } + + this.update((state) => { + // Update fields that weren't available pre-submission + const updatedItem: BridgeHistoryItem = { + ...historyItem, + txMetaId: txMeta.id, + originalTransactionId: historyItem.originalTransactionId ?? txMeta.id, + status: { + ...historyItem.status, + srcChain: { + ...historyItem.status.srcChain, + txHash: txMeta.hash ?? historyItem.status.srcChain?.txHash, + }, + }, + }; + + // Add under new key (txMeta.id) + state.txHistory[txMeta.id] = updatedItem; + // Remove old key (actionId) + delete state.txHistory[actionId]; }); }; @@ -532,6 +609,12 @@ export class BridgeStatusController extends StaticIntervalPollingController { const { bridgeTxMeta } = txHistoryMeta; + if (!bridgeTxMeta?.id) { + throw new Error( + 'Cannot start polling: bridgeTxMeta.id is required for polling', + ); + } + this.#addTxToHistory(txHistoryMeta); this.#startPollingForTxId(bridgeTxMeta.id); }; @@ -1157,6 +1240,7 @@ export class BridgeStatusController extends StaticIntervalPollingController => { - const actionId = generateActionId().toString(); + // Use provided actionId (for pre-submission history) or generate one + const actionId = providedActionId ?? generateActionId().toString(); const selectedAccount = this.messenger.call( 'AccountsController:getAccountByAddress', @@ -1534,9 +1621,30 @@ export class BridgeStatusController extends StaticIntervalPollingController; export type RefuelStatusResponse = object & StatusResponse; export type BridgeHistoryItem = { - txMetaId: string; // Need this to handle STX that might not have a txHash immediately + txMetaId?: string; // Optional: not available pre-submission or on sync failure + actionId?: string; // Only for non-batch EVM transactions originalTransactionId?: string; // Keep original transaction ID for intent transactions batchId?: string; quote: Quote; @@ -195,7 +196,7 @@ export type QuoteMetadataSerialized = { }; export type StartPollingForBridgeTxStatusArgs = { - bridgeTxMeta: TransactionMeta; + bridgeTxMeta?: TransactionMeta; statusRequest: StatusRequest; quoteResponse: QuoteResponse & QuoteMetadata; startTime?: BridgeHistoryItem['startTime']; diff --git a/packages/bridge-status-controller/src/utils/transaction.test.ts b/packages/bridge-status-controller/src/utils/transaction.test.ts index eb6e075e1ea..39174201c76 100644 --- a/packages/bridge-status-controller/src/utils/transaction.test.ts +++ b/packages/bridge-status-controller/src/utils/transaction.test.ts @@ -24,6 +24,8 @@ import { toBatchTxParams, getAddTransactionBatchParams, findAndUpdateTransactionsInBatch, + getHistoryKey, + getIntentFromQuote, } from './transaction'; import { APPROVAL_DELAY_MS } from '../constants'; import type { BridgeStatusControllerMessenger } from '../types'; @@ -2037,4 +2039,66 @@ describe('Bridge Status Controller Transaction Utils', () => { expect(mockUpdateTransactionFn).not.toHaveBeenCalled(); }); }); + + describe('getHistoryKey', () => { + it('returns actionId when both actionId and bridgeTxMetaId are provided', () => { + expect(getHistoryKey('action-123', 'tx-456')).toBe('action-123'); + }); + + it('returns bridgeTxMetaId when only bridgeTxMetaId is provided', () => { + expect(getHistoryKey(undefined, 'tx-456')).toBe('tx-456'); + }); + + it('returns actionId when only actionId is provided', () => { + expect(getHistoryKey('action-123', undefined)).toBe('action-123'); + }); + + it('throws error when neither actionId nor bridgeTxMetaId is provided', () => { + expect(() => getHistoryKey(undefined, undefined)).toThrow( + 'Cannot add tx to history: either actionId or bridgeTxMeta.id must be provided', + ); + }); + }); + + describe('getIntentFromQuote', () => { + it('returns intent when present in quote response', () => { + const mockIntent = { protocol: 'cowswap', order: { some: 'data' } }; + const quoteResponse = { + quote: { + intent: mockIntent, + srcChainId: 1, + destChainId: 1, + }, + } as never; + + expect(getIntentFromQuote(quoteResponse)).toBe(mockIntent); + }); + + it('throws error when intent is missing from quote', () => { + const quoteResponse = { + quote: { + srcChainId: 1, + destChainId: 1, + }, + } as never; + + expect(() => getIntentFromQuote(quoteResponse)).toThrow( + 'submitIntent: missing intent data', + ); + }); + + it('throws error when intent is undefined', () => { + const quoteResponse = { + quote: { + intent: undefined, + srcChainId: 1, + destChainId: 1, + }, + } as never; + + expect(() => getIntentFromQuote(quoteResponse)).toThrow( + 'submitIntent: missing intent data', + ); + }); + }); }); diff --git a/packages/bridge-status-controller/src/utils/transaction.ts b/packages/bridge-status-controller/src/utils/transaction.ts index 63248853447..ed0cf176176 100644 --- a/packages/bridge-status-controller/src/utils/transaction.ts +++ b/packages/bridge-status-controller/src/utils/transaction.ts @@ -8,6 +8,7 @@ import { isCrossChain, } from '@metamask/bridge-controller'; import type { + Intent, QuoteMetadata, QuoteResponse, Trade, @@ -453,3 +454,42 @@ export const findAndUpdateTransactionsInBatch = ({ return txBatch; }; + +/** + * Determines the key to use for storing a bridge history item. + * Uses actionId for pre-submission tracking, or bridgeTxMetaId for post-submission. + * + * @param actionId - The action ID used for pre-submission tracking + * @param bridgeTxMetaId - The transaction meta ID from bridgeTxMeta + * @returns The key to use for the history item + * @throws Error if neither actionId nor bridgeTxMetaId is provided + */ +export function getHistoryKey( + actionId: string | undefined, + bridgeTxMetaId: string | undefined, +): string { + const historyKey = actionId ?? bridgeTxMetaId; + if (!historyKey) { + throw new Error( + 'Cannot add tx to history: either actionId or bridgeTxMeta.id must be provided', + ); + } + return historyKey; +} + +/** + * Extracts and validates the intent data from a quote response. + * + * @param quoteResponse - The quote response that may contain intent data + * @returns The intent data from the quote + * @throws Error if the quote does not contain intent data + */ +export function getIntentFromQuote( + quoteResponse: QuoteResponse & { quote: { intent?: Intent } }, +): Intent { + const { intent } = quoteResponse.quote; + if (!intent) { + throw new Error('submitIntent: missing intent data'); + } + return intent; +}