diff --git a/packages/bridge-controller/CHANGELOG.md b/packages/bridge-controller/CHANGELOG.md index cf4e7a990f6..7528658d2c0 100644 --- a/packages/bridge-controller/CHANGELOG.md +++ b/packages/bridge-controller/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `AccountHardwareType` type and `getAccountHardwareType` function to the package exports ([#8503](https://github.com/MetaMask/core/pull/8503)) - `AccountHardwareType` is a union of `'Ledger' | 'Trezor' | 'QR Hardware' | 'Lattice' | null` - `getAccountHardwareType` maps a keyring type string to the corresponding `AccountHardwareType` value +- Read 'maxPendingHistoryItemAgeMs' feature flag from LaunchDarkly, which indicates when a history item can be treated as a failure ([#8479](https://github.com/MetaMask/core/pull/8479)) +- Add the `invalid_transaction_hash` polling reason to indicate that a history item was removed from state do to having an invalid hash ([#8479](https://github.com/MetaMask/core/pull/8479)) ### Changed diff --git a/packages/bridge-controller/src/utils/metrics/constants.ts b/packages/bridge-controller/src/utils/metrics/constants.ts index 088d4bdb8e7..85de258c851 100644 --- a/packages/bridge-controller/src/utils/metrics/constants.ts +++ b/packages/bridge-controller/src/utils/metrics/constants.ts @@ -27,6 +27,7 @@ export enum UnifiedSwapBridgeEventName { export enum PollingStatus { MaxPollingReached = 'max_polling_reached', + InvalidTransactionHash = 'invalid_transaction_hash', ManuallyRestarted = 'manually_restarted', } diff --git a/packages/bridge-controller/src/utils/validators.ts b/packages/bridge-controller/src/utils/validators.ts index 73603b2ee9b..5e21a41779a 100644 --- a/packages/bridge-controller/src/utils/validators.ts +++ b/packages/bridge-controller/src/utils/validators.ts @@ -189,6 +189,7 @@ export const PlatformConfigSchema = type({ * Array of chain objects ordered by preference/ranking */ chainRanking: ChainRankingSchema, + maxPendingHistoryItemAgeMs: optional(number()), }); export const validateFeatureFlagsResponse = ( diff --git a/packages/bridge-status-controller/CHANGELOG.md b/packages/bridge-status-controller/CHANGELOG.md index 4ad04f92c73..feb61b7f54a 100644 --- a/packages/bridge-status-controller/CHANGELOG.md +++ b/packages/bridge-status-controller/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Remove stale bridge transactions from `txHistory` to prevent excessive polling. Once a history item exceeds the configured maximum age, the status is fetched once, then the src tx hash's receipt is retrieved. If there is no receipt, the history item's hash is presumed to be invalid and the entry is deleted from state. ([#8479](https://github.com/MetaMask/core/pull/8479)) - Add missing action types for public `BridgeStatusController` methods ([#8367](https://github.com/MetaMask/core/pull/8367)) - The following types are now available: - `BridgeStatusControllerSubmitTxAction` @@ -17,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING:** Replace `transactionFailed` and `transactionConfirmed` event subscriptions with `TransactionController:transactionStatusUpdated` ([#8479](https://github.com/MetaMask/core/pull/8479)) +- **BREAKING:** Add `RemoteFeatureFlags:getState` to allowed actions to retrieve max history item age config ([#8479](https://github.com/MetaMask/core/pull/8479)) - Add `account_hardware_type` field to all cross-chain swap analytics events ([#8503](https://github.com/MetaMask/core/pull/8503)) - `account_hardware_type` carries the specific hardware wallet brand (e.g. `'Ledger'`, `'QR Hardware'`) or `null` for software wallets - `is_hardware_wallet` is now derived from `account_hardware_type !== null`, keeping both fields in sync @@ -28,6 +31,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/base-controller` from `^9.0.1` to `^9.1.0` ([#8457](https://github.com/MetaMask/core/pull/8457)) - Bump `@metamask/bridge-controller` from `^70.0.1` to `^70.1.1` ([#8466](https://github.com/MetaMask/core/pull/8466), [#8474](https://github.com/MetaMask/core/pull/8474)) +### Fixed + +- Prevent invalid src hashes from being persisted in `txHistory` ([#8479](https://github.com/MetaMask/core/pull/8479)) + - Make transaction status subscribers generic so that `txHistory` items get updated if there are any transaction updates matching by actionId, txMetaId, hash or type + - Skip saving smart transaction hashes on transaction submission. This used to make it possible for invalid src hashes to be stored in state and polled indefinitely. Instead, the txHistory item will now be updated with the confirmed tx hash when the `transactionStatusUpdated` event is published + - If there is no srcTxHash in state, attempt to set it based on the local TransactionController state + ## [70.0.5] ### 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 7230ce5ee71..f6a76240968 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 @@ -145,6 +145,60 @@ exports[`BridgeStatusController constructor rehydrates the tx history state 1`] } `; +exports[`BridgeStatusController constructor when history has no tx hash, has txMeta and is older than 2 days 1`] = ` +[ + { + "method": "eth_getTransactionReceipt", + "params": [ + "0xsrcTxHash3", + ], + }, +] +`; + +exports[`BridgeStatusController constructor when history has no tx hash, no txMeta and is older than 2 days 1`] = `[]`; + +exports[`BridgeStatusController constructor when history has no txHash, no txMeta, provider returns no receipt and is older than 2 days 1`] = `[]`; + +exports[`BridgeStatusController constructor when history has no txHash, no txMeta, provider returns receipt and is older than 2 days 1`] = `[]`; + +exports[`BridgeStatusController constructor when history has no txHash, no txMeta, provider throws error and is older than 2 days 1`] = `[]`; + +exports[`BridgeStatusController constructor when history has solana srcChainId, no tx hash, no txMeta and is older than 2 days 1`] = `[]`; + +exports[`BridgeStatusController constructor when history has tx hash, provider returns no receipt and is older than 2 days 1`] = ` +[ + { + "method": "eth_getTransactionReceipt", + "params": [ + "0xsrcTxHash2", + ], + }, +] +`; + +exports[`BridgeStatusController constructor when history has tx hash, provider returns receipt and is older than 2 days 1`] = ` +[ + { + "method": "eth_getTransactionReceipt", + "params": [ + "0xsrcTxHash2", + ], + }, +] +`; + +exports[`BridgeStatusController constructor when history has tx hash, provider throws error and is older than 2 days 1`] = ` +[ + { + "method": "eth_getTransactionReceipt", + "params": [ + "0xsrcTxHash2", + ], + }, +] +`; + exports[`BridgeStatusController startPollingForBridgeTxStatus emits bridgeTransactionFailed event when the status response is failed 1`] = ` [ [ @@ -327,6 +381,9 @@ exports[`BridgeStatusController startPollingForBridgeTxStatus sets the inital tx exports[`BridgeStatusController startPollingForBridgeTxStatus stops polling when the status response is complete 1`] = ` [ + [ + "TransactionController:getState", + ], [ "AuthenticationController:getBearerToken", ], @@ -1165,7 +1222,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should handle smart transac "status": { "srcChain": { "chainId": 42161, - "txHash": "0xevmTxHash", + "txHash": undefined, }, "status": "PENDING", }, @@ -3708,7 +3765,7 @@ exports[`BridgeStatusController submitTx: EVM swap should handle a gasless swap "status": { "srcChain": { "chainId": 42161, - "txHash": "0xevmTxHash", + "txHash": undefined, }, "status": "PENDING", }, @@ -3853,7 +3910,7 @@ exports[`BridgeStatusController submitTx: EVM swap should handle smart transacti "status": { "srcChain": { "chainId": 42161, - "txHash": "0xevmTxHash", + "txHash": undefined, }, "status": "PENDING", }, @@ -6036,30 +6093,57 @@ exports[`BridgeStatusController submitTx: Tron swap with approval should success } `; -exports[`BridgeStatusController subscription handlers TransactionController:transactionConfirmed should not start polling for bridge tx if tx is not in txHistory 1`] = `[]`; +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (confirmed) should not start polling for bridge tx if tx is not in txHistory 1`] = `[]`; -exports[`BridgeStatusController subscription handlers TransactionController:transactionConfirmed should not track completed event for other transaction types 1`] = `[]`; +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (confirmed) should not track completed event for other transaction types 1`] = `[]`; -exports[`BridgeStatusController subscription handlers TransactionController:transactionConfirmed should start polling for bridge tx if status response is invalid 1`] = ` +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (confirmed) should start polling for bridge tx if status response is invalid 1`] = ` [ - "BridgeController:trackUnifiedSwapBridgeEvent", - "Unified SwapBridge Status Failed Validation", - { - "action_type": "swapbridge-v1", - "chain_id_destination": "eip155:10", - "chain_id_source": "eip155:42161", - "failures": [ - "across|unknown", - ], - "location": "Main View", - "refresh_count": 0, - "token_address_destination": "eip155:10/slip44:60", - "token_address_source": "eip155:42161/slip44:60", - }, + [ + "AuthenticationController:getBearerToken", + ], + [ + "AuthenticationController:getBearerToken", + ], + [ + "AuthenticationController:getBearerToken", + ], + [ + "BridgeController:trackUnifiedSwapBridgeEvent", + "Unified SwapBridge Status Failed Validation", + { + "action_type": "swapbridge-v1", + "chain_id_destination": "eip155:10", + "chain_id_source": "eip155:42161", + "failures": [ + "across|status", + ], + "location": "Main View", + "refresh_count": 0, + "token_address_destination": "eip155:10/slip44:60", + "token_address_source": "eip155:42161/slip44:60", + }, + ], + [ + "BridgeController:trackUnifiedSwapBridgeEvent", + "Unified SwapBridge Status Failed Validation", + { + "action_type": "swapbridge-v1", + "chain_id_destination": "eip155:10", + "chain_id_source": "eip155:42161", + "failures": [ + "across|unknown", + ], + "location": "Main View", + "refresh_count": 0, + "token_address_destination": "eip155:10/slip44:60", + "token_address_source": "eip155:42161/slip44:60", + }, + ], ] `; -exports[`BridgeStatusController subscription handlers TransactionController:transactionConfirmed should start polling for bridge tx if status response is invalid 2`] = ` +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (confirmed) should start polling for bridge tx if status response is invalid 2`] = ` [ [ "Failed to fetch bridge tx status", @@ -6072,7 +6156,7 @@ exports[`BridgeStatusController subscription handlers TransactionController:tran ] `; -exports[`BridgeStatusController subscription handlers TransactionController:transactionConfirmed should start polling for completed bridge tx with featureId 2`] = ` +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (confirmed) should start polling for completed bridge tx with featureId 2`] = ` { "bridge": "across", "destChain": { @@ -6113,7 +6197,7 @@ exports[`BridgeStatusController subscription handlers TransactionController:tran } `; -exports[`BridgeStatusController subscription handlers TransactionController:transactionConfirmed should start polling for failed bridge tx with featureId 2`] = ` +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (confirmed) should start polling for failed bridge tx with featureId 2`] = ` { "bridge": "debridge", "destChain": { @@ -6140,7 +6224,7 @@ exports[`BridgeStatusController subscription handlers TransactionController:tran } `; -exports[`BridgeStatusController subscription handlers TransactionController:transactionConfirmed should track completed event for swap transaction 1`] = ` +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (confirmed) should track completed event for swap transaction 1`] = ` [ [ "AccountsController:getAccountByAddress", @@ -6190,7 +6274,7 @@ exports[`BridgeStatusController subscription handlers TransactionController:tran ] `; -exports[`BridgeStatusController subscription handlers TransactionController:transactionFailed should find history by actionId when txMeta.id not in history (pre-submission failure) 1`] = ` +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (failed) should find history by actionId when txMeta.id not in history (pre-submission failure) 1`] = ` [ "BridgeController:trackUnifiedSwapBridgeEvent", "Unified SwapBridge Failed", @@ -6204,7 +6288,7 @@ exports[`BridgeStatusController subscription handlers TransactionController:tran "chain_id_source": "eip155:42161", "custom_slippage": true, "destination_transaction": "FAILED", - "error_message": "", + "error_message": "Transaction failed. tx-error", "gas_included": false, "gas_included_7702": false, "is_hardware_wallet": false, @@ -6232,13 +6316,13 @@ exports[`BridgeStatusController subscription handlers TransactionController:tran ] `; -exports[`BridgeStatusController subscription handlers TransactionController:transactionFailed should not track failed event for approved status 1`] = `[]`; +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (failed) should not track failed event for approved status 1`] = `[]`; -exports[`BridgeStatusController subscription handlers TransactionController:transactionFailed should not track failed event for other transaction types 1`] = `[]`; +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (failed) should not track failed event for other transaction types 1`] = `[]`; -exports[`BridgeStatusController subscription handlers TransactionController:transactionFailed should not track failed event for signed status 1`] = `[]`; +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (failed) should not track failed event for signed status 1`] = `[]`; -exports[`BridgeStatusController subscription handlers TransactionController:transactionFailed should track failed event for bridge transaction 1`] = ` +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (failed) should track failed event for bridge transaction 1`] = ` [ "BridgeController:trackUnifiedSwapBridgeEvent", "Unified SwapBridge Failed", @@ -6252,7 +6336,7 @@ exports[`BridgeStatusController subscription handlers TransactionController:tran "chain_id_source": "eip155:42161", "custom_slippage": true, "destination_transaction": "FAILED", - "error_message": "", + "error_message": "Transaction failed. tx-error", "gas_included": false, "gas_included_7702": false, "is_hardware_wallet": false, @@ -6280,45 +6364,58 @@ exports[`BridgeStatusController subscription handlers TransactionController:tran ] `; -exports[`BridgeStatusController subscription handlers TransactionController:transactionFailed should track failed event for bridge transaction if approval is dropped 1`] = ` +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (failed) should track failed event for bridge transaction if approval is dropped 1`] = ` [ - "BridgeController:trackUnifiedSwapBridgeEvent", - "Unified SwapBridge Failed", - { - "account_hardware_type": null, - "action_type": "swapbridge-v1", - "actual_time_minutes": 0, - "chain_id_destination": "eip155:42161", - "chain_id_source": "eip155:42161", - "custom_slippage": false, - "error_message": "", - "gas_included": false, - "gas_included_7702": false, - "is_hardware_wallet": false, - "location": "Main View", - "price_impact": 0, - "provider": "", - "quote_vs_execution_ratio": 0, - "quoted_time_minutes": 0, - "quoted_vs_used_gas_ratio": 0, - "security_warnings": [], - "source_transaction": "FAILED", - "stx_enabled": false, - "swap_type": "crosschain", - "token_address_destination": "eip155:42161/slip44:60", - "token_address_source": "eip155:42161/slip44:60", - "token_symbol_destination": "", - "token_symbol_source": "", - "usd_actual_gas": 0, - "usd_actual_return": 0, - "usd_amount_source": 0, - "usd_quoted_gas": 0, - "usd_quoted_return": 0, - }, + [ + "AccountsController:getAccountByAddress", + "0xaccount1", + ], + [ + "TransactionController:getState", + ], + [ + "BridgeController:trackUnifiedSwapBridgeEvent", + "Unified SwapBridge Failed", + { + "account_hardware_type": null, + "action_type": "swapbridge-v1", + "actual_time_minutes": 0, + "allowance_reset_transaction": undefined, + "approval_transaction": "COMPLETE", + "chain_id_destination": "eip155:10", + "chain_id_source": "eip155:42161", + "custom_slippage": true, + "destination_transaction": "FAILED", + "error_message": "Transaction dropped. tx-error", + "gas_included": false, + "gas_included_7702": false, + "is_hardware_wallet": false, + "location": "Main View", + "price_impact": 0, + "provider": "lifi_across", + "quote_vs_execution_ratio": 0, + "quoted_time_minutes": 0.25, + "quoted_vs_used_gas_ratio": 0, + "security_warnings": [], + "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 track failed event for bridge transaction if not in txHistory 1`] = ` +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (failed) should track failed event for bridge transaction if not in txHistory 1`] = ` [ [ "BridgeController:trackUnifiedSwapBridgeEvent", @@ -6330,7 +6427,7 @@ exports[`BridgeStatusController subscription handlers TransactionController:tran "chain_id_destination": "eip155:42161", "chain_id_source": "eip155:42161", "custom_slippage": false, - "error_message": "", + "error_message": "Transaction failed. tx-error", "gas_included": false, "gas_included_7702": false, "is_hardware_wallet": false, @@ -6358,7 +6455,7 @@ exports[`BridgeStatusController subscription handlers TransactionController:tran ] `; -exports[`BridgeStatusController subscription handlers TransactionController:transactionFailed should track failed event for swap transaction 1`] = ` +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (failed) should track failed event for swap transaction 1`] = ` [ [ "AccountsController:getAccountByAddress", @@ -6380,7 +6477,7 @@ exports[`BridgeStatusController subscription handlers TransactionController:tran "chain_id_source": "eip155:42161", "custom_slippage": true, "destination_transaction": "FAILED", - "error_message": "", + "error_message": "Transaction failed. tx-error", "gas_included": false, "gas_included_7702": false, "is_hardware_wallet": false, @@ -6409,7 +6506,7 @@ exports[`BridgeStatusController subscription handlers TransactionController:tran ] `; -exports[`BridgeStatusController subscription handlers TransactionController:transactionFailed should track failed event for swap transaction if approval fails 1`] = ` +exports[`BridgeStatusController subscription handlers TransactionController:transactionStatusUpdated (failed) should track failed event for swap transaction if approval fails 1`] = ` [ "BridgeController:trackUnifiedSwapBridgeEvent", "Unified SwapBridge Failed", @@ -6417,31 +6514,35 @@ exports[`BridgeStatusController subscription handlers TransactionController:tran "account_hardware_type": null, "action_type": "swapbridge-v1", "actual_time_minutes": 0, - "chain_id_destination": "eip155:42161", + "allowance_reset_transaction": undefined, + "approval_transaction": "COMPLETE", + "chain_id_destination": "eip155:10", "chain_id_source": "eip155:42161", - "custom_slippage": false, - "error_message": "", + "custom_slippage": true, + "destination_transaction": "FAILED", + "error_message": "Transaction failed. approval-tx-error", "gas_included": false, "gas_included_7702": false, "is_hardware_wallet": false, "location": "Main View", "price_impact": 0, - "provider": "", + "provider": "lifi_across", "quote_vs_execution_ratio": 0, - "quoted_time_minutes": 0, + "quoted_time_minutes": 0.25, "quoted_vs_used_gas_ratio": 0, "security_warnings": [], - "source_transaction": "FAILED", + "slippage_limit": 0, + "source_transaction": "COMPLETE", "stx_enabled": false, - "swap_type": "single_chain", - "token_address_destination": "eip155:42161/slip44:60", + "swap_type": "crosschain", + "token_address_destination": "eip155:10/slip44:60", "token_address_source": "eip155:42161/slip44:60", - "token_symbol_destination": "", - "token_symbol_source": "", + "token_symbol_destination": "ETH", + "token_symbol_source": "ETH", "usd_actual_gas": 0, "usd_actual_return": 0, "usd_amount_source": 0, - "usd_quoted_gas": 0, + "usd_quoted_gas": 2.5778, "usd_quoted_return": 0, }, ] diff --git a/packages/bridge-status-controller/src/bridge-status-controller.intent.test.ts b/packages/bridge-status-controller/src/bridge-status-controller.intent.test.ts index 46dcad4c88e..7acf911f698 100644 --- a/packages/bridge-status-controller/src/bridge-status-controller.intent.test.ts +++ b/packages/bridge-status-controller/src/bridge-status-controller.intent.test.ts @@ -730,169 +730,6 @@ describe('BridgeStatusController (subscriptions + bridge polling + wiping)', () jest.clearAllMocks(); }); - it('transactionFailed subscription: marks main tx as FAILED and tracks (non-rejected)', async () => { - const mockTxHistory = { - bridgeTxMetaId1: { - txMetaId: 'bridgeTxMetaId1', - originalTransactionId: 'bridgeTxMetaId1', - quote: { - srcChainId: 1, - destChainId: 10, - srcAsset: { - address: '0x0000000000000000000000000000000000000000', - assetId: 'eip155:1/slip44:60', - }, - destAsset: { assetId: 'eip155:10/slip44:60' }, - bridges: ['across'], - bridgeId: 'rango', - }, - account: '0xAccount1', - status: { - status: StatusTypes.PENDING, - srcChain: { chainId: 1, txHash: '0xsrc' }, - }, - }, - }; - const { controller, messenger } = setup({ - mockTxHistory, - }); - - const failedCb = messenger.subscribe.mock.calls.find( - ([evt]: [any]) => evt === 'TransactionController:transactionFailed', - )?.[1]; - expect(typeof failedCb).toBe('function'); - - failedCb({ - transactionMeta: { - id: 'bridgeTxMetaId1', - type: TransactionType.bridge, - status: TransactionStatus.failed, - chainId: '0x1', - }, - }); - - controller.stopAllPolling(); - - expect(controller.state.txHistory.bridgeTxMetaId1.status.status).toBe( - StatusTypes.FAILED, - ); - - // ensure tracking was attempted - expect((messenger.call as jest.Mock).mock.calls).toStrictEqual( - expect.arrayContaining([ - expect.arrayContaining([ - 'BridgeController:trackUnifiedSwapBridgeEvent', - ]), - ]), - ); - }); - - it('transactionFailed subscription: maps approval tx id back to main history item', async () => { - const mockTxHistory = { - mainTx: { - txMetaId: 'mainTx', - originalTransactionId: 'mainTx', - approvalTxId: 'approvalTx', - quote: { - srcChainId: 1, - destChainId: 10, - srcAsset: { - address: '0x0000000000000000000000000000000000000000', - assetId: 'eip155:1/slip44:60', - }, - destAsset: { - address: '0x0000000000000000000000000000000000000000', - assetId: 'eip155:10/slip44:60', - }, - bridges: ['cowswap'], - bridgeId: 'cowswap', - }, - account: '0xAccount1', - status: { - status: StatusTypes.PENDING, - srcChain: { chainId: 1, txHash: '0xsrc' }, - }, - }, - }; - const { controller, messenger } = setup({ - mockTxHistory, - }); - const failedCb = messenger.subscribe.mock.calls.find( - ([evt]: [any]) => evt === 'TransactionController:transactionFailed', - )?.[1]; - - failedCb({ - transactionMeta: { - id: 'approvalTx', - type: TransactionType.bridgeApproval, - status: TransactionStatus.failed, - chainId: '0x1', - }, - }); - - controller.stopAllPolling(); - - expect(controller.state.txHistory.mainTx.status.status).toBe( - StatusTypes.FAILED, - ); - }); - - it('transactionConfirmed subscription: tracks swap Completed; starts polling on bridge confirmed', async () => { - const mockTxHistory = { - bridgeConfirmed1: { - txMetaId: 'bridgeConfirmed1', - originalTransactionId: 'bridgeConfirmed1', - quote: { - srcChainId: 1, - destChainId: 10, - srcAsset: { - address: '0x0000000000000000000000000000000000000000', - assetId: 'eip155:1/slip44:60', - }, - destAsset: { - address: '0x0000000000000000000000000000000000000000', - assetId: 'eip155:10/slip44:60', - }, - bridges: ['cowswap'], - bridgeId: 'cowswap', - }, - account: '0xAccount1', - status: { - status: StatusTypes.PENDING, - srcChain: { chainId: 1, txHash: '0xsrc' }, - }, - }, - }; - const { messenger, controller, startPollingSpy } = setup({ - mockTxHistory, - }); - - const confirmedCb = messenger.subscribe.mock.calls.find( - ([evt]: [any]) => evt === 'TransactionController:transactionConfirmed', - )?.[1]; - expect(typeof confirmedCb).toBe('function'); - - // Swap -> Completed tracking - confirmedCb({ - id: 'swap1', - type: TransactionType.swap, - chainId: '0x1', - }); - - // Bridge -> startPolling - confirmedCb({ - id: 'bridgeConfirmed1', - type: TransactionType.bridge, - chainId: '0x1', - }); - - controller.stopAllPolling(); - - expect(startPollingSpy).toHaveBeenCalledWith({ - bridgeTxMetaId: 'bridgeConfirmed1', - }); - }); - it('restartPollingForFailedAttempts: throws when identifier missing, and when no match found', async () => { const { controller } = setup(); @@ -1021,43 +858,6 @@ describe('BridgeStatusController (target uncovered branches)', () => { jest.clearAllMocks(); }); - it('transactionFailed: returns early for intent txs (swapMetaData.isIntentTx)', () => { - const mockTxHistory = { - tx1: { - txMetaId: 'tx1', - originalTransactionId: 'tx1', - quote: minimalIntentQuoteResponse().quote, - account: '0xAccount1', - status: { - status: StatusTypes.PENDING, - srcChain: { chainId: 1, txHash: '0x' }, - }, - }, - }; - const { controller, messenger } = setup({ - mockTxHistory, - }); - - const failedCb = messenger.subscribe.mock.calls.find( - ([evt]: [any]) => evt === 'TransactionController:transactionFailed', - )?.[1]; - - failedCb({ - transactionMeta: { - id: 'tx1', - chainId: '0x1', - type: TransactionType.bridge, - status: TransactionStatus.failed, - swapMetaData: { isIntentTx: true }, // <- triggers early return - }, - }); - - expect(controller.state.txHistory.tx1.status.status).toBe( - StatusTypes.FAILED, - ); - controller.stopAllPolling(); - }); - it('constructor restartPolling: skips items when shouldSkipFetchDueToFetchFailures returns true', () => { const accountAddress = '0xAccount1'; const { messenger } = createMessengerHarness(accountAddress); @@ -1227,6 +1027,7 @@ describe('BridgeStatusController (target uncovered branches)', () => { status: StatusTypes.PENDING, srcChain: { chainId: 1, txHash: '0xhash' }, }, + startTime: Date.now() - 1000, }, }, }); @@ -1243,6 +1044,9 @@ describe('BridgeStatusController (target uncovered branches)', () => { expect(messenger.call.mock.calls).toMatchInlineSnapshot(` [ + [ + "RemoteFeatureFlagController:getState", + ], [ "AuthenticationController:getBearerToken", ], @@ -1286,7 +1090,8 @@ describe('BridgeStatusController (target uncovered branches)', () => { }); const failedCb = messenger.subscribe.mock.calls.find( - ([evt]: [any]) => evt === 'TransactionController:transactionFailed', + ([evt]: [any]) => + evt === 'TransactionController:transactionStatusUpdated', )?.[1]; failedCb({ 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 cdf6df3e66f..d9f51b6a7bf 100644 --- a/packages/bridge-status-controller/src/bridge-status-controller.test.ts +++ b/packages/bridge-status-controller/src/bridge-status-controller.test.ts @@ -24,6 +24,7 @@ import type { MessengerEvents, MockAnyNamespace, } from '@metamask/messenger'; +import type { Provider } from '@metamask/network-controller'; import { CHAIN_IDS } from '@metamask/transaction-controller'; import { TransactionType, @@ -41,6 +42,7 @@ import { BridgeStatusController } from './bridge-status-controller'; import { BRIDGE_STATUS_CONTROLLER_NAME, DEFAULT_BRIDGE_STATUS_CONTROLLER_STATE, + DEFAULT_MAX_PENDING_HISTORY_ITEM_AGE_MS, MAX_ATTEMPTS, } from './constants'; import { BridgeClientId } from './types'; @@ -53,6 +55,7 @@ import type { StatusResponse, } from './types'; import * as bridgeStatusUtils from './utils/bridge-status'; +import * as historyUtils from './utils/history'; import * as transactionUtils from './utils/transaction'; type AllBridgeStatusControllerActions = @@ -94,7 +97,7 @@ const MockStatusResponse = { status: 'PENDING' as StatusTypes, srcChain: { chainId: srcChainId, - txHash: srcTxHash, + txHash: srcTxHash === 'undefined' ? undefined : srcTxHash, amount: '991250000000000', token: { address: '0x0000000000000000000000000000000000000000', @@ -338,6 +341,7 @@ const MockTxHistory = { account = '0xaccount1', srcChainId = 42161, destChainId = 10, + srcTxHash = '0xsrcTxHash1', } = {}): Record => ({ [txMetaId]: { txMetaId, @@ -352,6 +356,7 @@ const MockTxHistory = { initialDestAssetBalance: undefined, pricingData: { amountSent: '1.234' }, status: MockStatusResponse.getPending({ + srcTxHash, srcChainId, }), hasApprovalTx: false, @@ -395,6 +400,8 @@ const MockTxHistory = { srcChainId = 42161, destChainId = 10, featureId = undefined, + attempts = undefined as BridgeHistoryItem['attempts'], + startTime = 1729964825189, } = {}): Record => ({ [txMetaId]: { txMetaId, @@ -402,7 +409,7 @@ const MockTxHistory = { originalTransactionId: txMetaId, batchId, quote: getMockQuote({ srcChainId, destChainId }), - startTime: 1729964825189, + startTime, estimatedProcessingTimeInSeconds: 15, slippagePercentage: 0, account, @@ -423,7 +430,7 @@ const MockTxHistory = { isStxEnabled: false, hasApprovalTx: false, completionTime: undefined, - attempts: undefined, + attempts, featureId, location: undefined, }, @@ -474,13 +481,14 @@ const MockTxHistory = { srcChainId = 42161, destChainId = 42161, featureId = undefined, + startTime = 1729964825189, } = {}): Record => ({ [txMetaId]: { txMetaId, actionId, originalTransactionId: txMetaId, quote: getMockQuote({ srcChainId, destChainId }), - startTime: 1729964825189, + startTime, estimatedProcessingTimeInSeconds: 15, slippagePercentage: 0, account, @@ -573,13 +581,11 @@ function getControllerMessenger( 'BridgeController:trackUnifiedSwapBridgeEvent', 'BridgeController:stopPollingForQuotes', 'GasFeeController:getState', + 'RemoteFeatureFlagController:getState', 'AuthenticationController:getBearerToken', 'KeyringController:signTypedMessage', ], - events: [ - 'TransactionController:transactionFailed', - 'TransactionController:transactionConfirmed', - ], + events: ['TransactionController:transactionStatusUpdated'], }); return messenger; } @@ -591,11 +597,21 @@ function registerDefaultActionHandlers( srcChainId = 42161, txHash = '0xsrcTxHash1', txMetaId = 'bridgeTxMetaId1', + status = TransactionStatus.confirmed, + provider = { + request: jest.fn().mockResolvedValue('0xreceipt1'), + sendAsync: jest.fn(), + send: jest.fn(), + }, + maxPendingHistoryItemAgeMs = DEFAULT_MAX_PENDING_HISTORY_ITEM_AGE_MS, }: { account?: string; srcChainId?: number; txHash?: string; txMetaId?: string; + status?: TransactionStatus; + provider?: Partial; + maxPendingHistoryItemAgeMs?: number; } = {}, ) { rootMessenger.registerActionHandler( @@ -626,12 +642,32 @@ function registerDefaultActionHandlers( configuration: { chainId: numberToHex(srcChainId), }, + // @ts-expect-error: Partial mock. + provider, }), ); rootMessenger.registerActionHandler('TransactionController:getState', () => ({ - transactions: [{ id: txMetaId, hash: txHash }], + // @ts-expect-error: Partial mock. + transactions: [{ id: txMetaId, hash: txHash, status }], })); + + rootMessenger.registerActionHandler( + 'RemoteFeatureFlagController:getState', + () => ({ + remoteFeatureFlags: { + bridgeConfig: { + maxPendingHistoryItemAgeMs, + }, + }, + cacheTimestamp: 1776474747215, + }), + ); + + rootMessenger.registerActionHandler( + 'AuthenticationController:getBearerToken', + () => Promise.resolve('auth-token'), + ); } type WithControllerCallback = (payload: { @@ -735,65 +771,298 @@ const mockSelectedAccount = { }, }; -describe('BridgeStatusController', () => { +describe('BridgeStatusController constructor', () => { beforeEach(() => { jest.clearAllMocks(); jest.clearAllTimers(); }); - describe('constructor', () => { - it('should setup correctly', async () => { - await withController(async ({ controller }) => { - expect(controller.state).toStrictEqual(EMPTY_INIT_STATE); - }); + it('should setup correctly', async () => { + await withController(async ({ controller }) => { + expect(controller.state).toStrictEqual(EMPTY_INIT_STATE); }); + }); - it('rehydrates the tx history state', async () => { - await withController( - { options: { state: { txHistory: MockTxHistory.getPending() } } }, - async ({ controller }) => { - // Assertion - expect(controller.state.txHistory).toMatchSnapshot(); - controller.stopAllPolling(); + it('rehydrates the tx history state', async () => { + await withController( + { options: { state: { txHistory: MockTxHistory.getPending() } } }, + async ({ controller }) => { + // Assertion + expect(controller.state.txHistory).toMatchSnapshot(); + controller.stopAllPolling(); + }, + ); + }); + + it('restarts polling for history items that are not complete', async () => { + // Setup + jest.useFakeTimers(); + const fetchBridgeTxStatusSpy = jest.spyOn( + bridgeStatusUtils, + 'fetchBridgeTxStatus', + ); + const provider = { + request: jest.fn().mockResolvedValueOnce('txReceipt1'), + }; + + jest.spyOn(historyUtils, 'isHistoryItemTooOld').mockReturnValue(false); + + await withController( + { + options: { + state: { + txHistory: { + ...MockTxHistory.getPending(), + ...MockTxHistory.getUnknown(), + ...MockTxHistory.getPendingSwap({ + srcTxHash: '0xswapSrcTxHash', + }), + ...MockTxHistory.getInitNoSrcTxHash({ + txMetaId: 'oldBridgeTxMetaId', + srcTxHash: '0xoldSrcTxHash', + }), + }, + }, + fetchFn: jest + .fn() + .mockResolvedValueOnce(MockStatusResponse.getPending()) + .mockResolvedValueOnce(MockStatusResponse.getComplete()) + .mockResolvedValueOnce(MockStatusResponse.getPending()), }, - ); - }); + }, + async ({ controller, rootMessenger }) => { + registerDefaultActionHandlers(rootMessenger, { + provider, + }); + const initialStatuses = { + bridgeTxMetaId1: 'PENDING', + bridgeTxMetaId2: 'UNKNOWN', + swapTxMetaId1: 'PENDING', + oldBridgeTxMetaId: 'PENDING', + }; + expect( + Object.entries(controller.state.txHistory).reduce( + (acc, [key, value]) => ({ + ...acc, + [key]: value.status.status, + }), + {}, + ), + ).toStrictEqual(initialStatuses); + + expect( + Object.entries(controller.state.txHistory).reduce( + (acc, [key, value]) => ({ + ...acc, + [key]: value.status.srcChain.txHash, + }), + {}, + ), + ).toMatchInlineSnapshot(` + { + "bridgeTxMetaId1": "0xsrcTxHash1", + "bridgeTxMetaId2": "0xsrcTxHash2", + "oldBridgeTxMetaId": "0xoldSrcTxHash", + "swapTxMetaId1": "0xswapSrcTxHash", + } + `); + + jest.advanceTimersByTime(10000); + await flushPromises(); + + // Assertions + expect(fetchBridgeTxStatusSpy).toHaveBeenCalledTimes(3); + expect(provider.request.mock.calls.flat()).toMatchInlineSnapshot(`[]`); + + expect(controller.state.txHistory.bridgeTxMetaId1.status.status).toBe( + StatusTypes.PENDING, + ); + expect(controller.state.txHistory.bridgeTxMetaId2.status.status).toBe( + StatusTypes.COMPLETE, + ); + expect(controller.state.txHistory.swapTxMetaId1.status.status).toBe( + StatusTypes.PENDING, + ); + expect(controller.state.txHistory.oldBridgeTxMetaId.status.status).toBe( + StatusTypes.PENDING, + ); + controller.stopAllPolling(); + }, + ); + }); - it('restarts polling for history items that are not complete', async () => { + it.each([ + { + title: 'tx hash, provider returns receipt', + txHash: '0xsrcTxHash2', + providerAction: async () => await Promise.resolve('txReceipt1'), + expectedHistoryTxMetaId: 'unknownTxMetaId1', + }, + { + title: 'tx hash, provider returns no receipt', + txHash: '0xsrcTxHash2', + providerAction: async () => await Promise.resolve(), + }, + { + title: 'tx hash, provider throws error', + txHash: '0xsrcTxHash2', + providerAction: async () => + await Promise.reject(new Error('Provider error')), + }, + { + title: 'no tx hash, has txMeta', + txMeta: { + id: 'txMetaId2', + hash: '0xsrcTxHash3', + }, + }, + { + title: 'no tx hash, no txMeta', + txMeta: { + id: 'undefined', + }, + }, + { + title: 'solana srcChainId, no tx hash, no txMeta', + txMeta: { + id: 'solanaTxMetaId1', + }, + srcChainId: ChainId.SOLANA, + expectedHistoryTxMetaId: 'solanaTxMetaId1', + txHash: 'solanaTxMetaId1', + }, + { + title: 'no txHash, no txMeta, provider returns no receipt', + txMeta: { + id: 'undefined', + }, + providerAction: async () => await Promise.resolve(), + }, + { + title: 'no txHash, no txMeta, provider returns receipt', + txMeta: { + id: 'undefined', + }, + providerAction: async () => await Promise.resolve('txReceipt1'), + }, + { + title: 'no txHash, no txMeta, provider throws error', + txMeta: { + id: 'undefined', + }, + providerAction: async () => + await Promise.reject(new Error('Provider error')), + }, + ])( + 'when history has $title and is older than 2 days', + async ({ + txHash = 'undefined', + providerAction = () => Promise.resolve(), + txMeta, + expectedHistoryTxMetaId, + srcChainId, + }) => { // Setup jest.useFakeTimers(); const fetchBridgeTxStatusSpy = jest.spyOn( bridgeStatusUtils, 'fetchBridgeTxStatus', ); + const provider = { + request: jest + .fn() + .mockImplementationOnce(async () => await providerAction()), + }; + const consoleWarnSpy = jest + .spyOn(console, 'warn') + .mockImplementationOnce(() => jest.fn()); + + const [historyKey, txHistoryItem] = Object.entries( + MockTxHistory.getPending({ + txMetaId: txMeta?.id ?? 'unknownTxMetaId1', + srcTxHash: txHash, + srcChainId, + }), + )[0]; + + const startTime = + Date.now() - DEFAULT_MAX_PENDING_HISTORY_ITEM_AGE_MS - 1000; await withController( { options: { state: { txHistory: { - ...MockTxHistory.getPending(), - ...MockTxHistory.getUnknown(), - ...MockTxHistory.getPendingSwap(), + [historyKey]: { + ...txHistoryItem, + attempts: { + counter: MAX_ATTEMPTS + 1, + lastAttemptTime: Date.now() - 1280000 - 1000, + }, + startTime, + }, }, }, fetchFn: jest .fn() - .mockResolvedValueOnce(MockStatusResponse.getPending()) - .mockResolvedValueOnce(MockStatusResponse.getComplete()), + .mockResolvedValueOnce(MockStatusResponse.getPending()), }, }, async ({ controller, rootMessenger }) => { - registerDefaultActionHandlers(rootMessenger); - jest.advanceTimersByTime(10000); + const stopPollingSpy = jest.spyOn( + controller, + 'stopPollingByPollingToken', + ); + const messengerCallSpy = jest.spyOn(rootMessenger, 'call'); + + registerDefaultActionHandlers(rootMessenger, { + provider, + txMetaId: txMeta?.id === 'undefined' ? undefined : txMeta?.id, + txHash: txMeta?.hash, + }); + controller.startPolling({ bridgeTxMetaId: historyKey }); + + expect( + controller.state.txHistory[historyKey].status.status, + ).toStrictEqual(StatusTypes.PENDING); + + jest.advanceTimersByTime(DEFAULT_MAX_PENDING_HISTORY_ITEM_AGE_MS); await flushPromises(); // Assertions - expect(fetchBridgeTxStatusSpy).toHaveBeenCalledTimes(2); - controller.stopAllPolling(); + expect(messengerCallSpy.mock.calls).toStrictEqual([]); + expect(fetchBridgeTxStatusSpy.mock.calls).toHaveLength(0); + expect(controller.state.txHistory[historyKey]?.txMetaId).toBe( + expectedHistoryTxMetaId, + ); + expect( + consoleWarnSpy.mock.calls.map((call) => JSON.stringify(call)), + ).toStrictEqual(['["Failed to fetch bridge tx status",{}]']); + + expect(controller.state.txHistory[historyKey]?.status.status).toBe( + expectedHistoryTxMetaId ? StatusTypes.PENDING : undefined, + ); + expect( + controller.state.txHistory[historyKey]?.attempts?.counter, + ).toBe(expectedHistoryTxMetaId ? MAX_ATTEMPTS + 2 : undefined); + expect(stopPollingSpy.mock.calls).toStrictEqual( + expectedHistoryTxMetaId ? [] : [['test-uuid-1234']], + ); + expect( + controller.state.txHistory[historyKey]?.status.srcChain.txHash, + ).toBe(expectedHistoryTxMetaId ? txHash : undefined); + expect(provider.request.mock.calls.flat()).toMatchSnapshot(); }, ); - }); + }, + ); +}); + +describe('BridgeStatusController', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.clearAllTimers(); + jest.spyOn(historyUtils, 'isHistoryItemTooOld').mockReturnValue(false); }); describe('startPolling - error handling', () => { @@ -900,10 +1169,12 @@ describe('BridgeStatusController', () => { } // Assertions - expect(fetchBridgeTxStatusSpy).toHaveBeenCalledTimes(MAX_ATTEMPTS); + expect(fetchBridgeTxStatusSpy).toHaveBeenCalledTimes( + MAX_ATTEMPTS + 1, + ); expect( - controller.state.txHistory.bridgeTxMetaId1.attempts?.counter, - ).toBe(MAX_ATTEMPTS); + controller.state.txHistory.bridgeTxMetaId1?.attempts?.counter, + ).toBe(MAX_ATTEMPTS + 1); // Verify polling stops after max attempts - even with a long wait, no more calls const callCountBeforeExtraTime = @@ -944,6 +1215,10 @@ describe('BridgeStatusController', () => { "Failed to fetch bridge tx status", [Error: Persistent error], ], + [ + "Failed to fetch bridge tx status", + [Error: Persistent error], + ], ] `); }, @@ -1041,14 +1316,16 @@ describe('BridgeStatusController', () => { }); await withController(async ({ controller, messenger, rootMessenger }) => { - registerDefaultActionHandlers(rootMessenger); + registerDefaultActionHandlers(rootMessenger, { + status: TransactionStatus.confirmed, + }); const messengerCallSpy = jest.spyOn(messenger, 'call'); const messengerPublishSpy = jest.spyOn(messenger, 'publish'); const fetchBridgeTxStatusSpy = jest.spyOn( bridgeStatusUtils, 'fetchBridgeTxStatus', ); - const stopPollingByNetworkClientIdSpy = jest.spyOn( + const stopPollingByPollingTokenSpy = jest.spyOn( controller, 'stopPollingByPollingToken', ); @@ -1068,7 +1345,8 @@ describe('BridgeStatusController', () => { await flushPromises(); // Assertions - expect(stopPollingByNetworkClientIdSpy).toHaveBeenCalledTimes(1); + expect(fetchBridgeTxStatusSpy).toHaveBeenCalledTimes(1); + expect(stopPollingByPollingTokenSpy).toHaveBeenCalledTimes(1); expect(controller.state.txHistory).toStrictEqual( MockTxHistory.getComplete(), ); @@ -1200,70 +1478,83 @@ describe('BridgeStatusController', () => { }); }); - it('updates the srcTxHash when one is available', async () => { - // Setup - jest.useFakeTimers(); - let getStateCallCount = 0; + it.each([ + { + status: TransactionStatus.confirmed, + }, + { status: TransactionStatus.failed }, + { status: TransactionStatus.dropped }, + { status: TransactionStatus.rejected }, + { status: TransactionStatus.signed, shouldSetSrcTxHash: false }, + ])( + 'updates the srcTxHash when one is available, with status %s', + async ({ status, shouldSetSrcTxHash = true }) => { + // Setup + jest.useFakeTimers(); + let getStateCallCount = 0; - await withController( - { - options: { - fetchFn: jest - .fn() - .mockResolvedValueOnce(MockStatusResponse.getPending()), - traceFn: jest.fn(), + await withController( + { + options: { + fetchFn: jest + .fn() + .mockResolvedValueOnce(MockStatusResponse.getPending()), + traceFn: jest.fn(), + }, }, - }, - async ({ controller, rootMessenger }) => { - registerDefaultActionHandlers(rootMessenger); + async ({ controller, rootMessenger }) => { + registerDefaultActionHandlers(rootMessenger); - rootMessenger.unregisterActionHandler( - 'TransactionController:getState', - ); - rootMessenger.registerActionHandler( - 'TransactionController:getState', - () => { - getStateCallCount += 1; - return { - transactions: [ - { - id: 'bridgeTxMetaId1', - hash: getStateCallCount === 0 ? undefined : '0xnewTxHash', - }, - ], - }; - }, - ); + rootMessenger.unregisterActionHandler( + 'TransactionController:getState', + ); + rootMessenger.registerActionHandler( + 'TransactionController:getState', + // @ts-expect-error: Partial mock. + () => { + getStateCallCount += 1; + return { + transactions: [ + { + id: 'bridgeTxMetaId1', + hash: getStateCallCount === 0 ? undefined : '0xnewTxHash', + status, + }, + ], + }; + }, + ); - // Start polling with no srcTxHash - const startPollingArgs = getMockStartPollingForBridgeTxStatusArgs({ - srcTxHash: 'undefined', - }); - rootMessenger.call( - 'BridgeStatusController:startPollingForBridgeTxStatus', - startPollingArgs, - ); + // Start polling with no srcTxHash + const startPollingArgs = getMockStartPollingForBridgeTxStatusArgs({ + srcTxHash: 'undefined', + }); + rootMessenger.call( + 'BridgeStatusController:startPollingForBridgeTxStatus', + startPollingArgs, + ); - // Verify initial state has no srcTxHash - expect( - controller.state.txHistory.bridgeTxMetaId1.status.srcChain.txHash, - ).toBeUndefined(); + // Verify initial state has no srcTxHash + expect( + controller.state.txHistory.bridgeTxMetaId1.status.srcChain.txHash, + ).toBeUndefined(); - // Advance timer to trigger polling with new hash - jest.advanceTimersByTime(10000); - await flushPromises(); + // Advance timer to trigger polling with new hash + jest.advanceTimersByTime(10000); + await flushPromises(); - // Verify the srcTxHash was updated - expect( - controller.state.txHistory.bridgeTxMetaId1.status.srcChain.txHash, - ).toBe('0xsrcTxHash1'); + // Verify the srcTxHash was updated + expect( + controller.state.txHistory.bridgeTxMetaId1.status.srcChain.txHash, + ).toBe(shouldSetSrcTxHash ? '0xsrcTxHash1' : undefined); - // Cleanup - controller.stopAllPolling(); - jest.restoreAllMocks(); - }, - ); - }); + // Cleanup + controller.stopAllPolling(); + jest.restoreAllMocks(); + }, + ); + }, + ); }); describe('resetState', () => { @@ -4613,11 +4904,9 @@ describe('BridgeStatusController', () => { 'TransactionController:getState', 'BridgeController:trackUnifiedSwapBridgeEvent', 'AccountsController:getAccountByAddress', + 'RemoteFeatureFlagController:getState', ], - events: [ - 'TransactionController:transactionFailed', - 'TransactionController:transactionConfirmed', - ], + events: ['TransactionController:transactionStatusUpdated'], }); jest @@ -4640,7 +4929,9 @@ describe('BridgeStatusController', () => { addTransactionBatchFn: jest.fn(), state: { txHistory: { - ...MockTxHistory.getPending(), + ...MockTxHistory.getPending({ + startTime: Date.now() - 1000, + }), ...MockTxHistory.getPendingSwap(), ...MockTxHistory.getPending({ txMetaId: 'bridgeTxMetaId1WithApproval', @@ -4649,11 +4940,13 @@ describe('BridgeStatusController', () => { ...MockTxHistory.getPendingSwap({ txMetaId: 'perpsSwapTxMetaId1', featureId: FeatureId.PERPS as never, + startTime: Date.now() - 1000, }), ...MockTxHistory.getPending({ txMetaId: 'perpsBridgeTxMetaId1', srcTxHash: '0xperpsSrcTxHash1', featureId: FeatureId.PERPS as never, + startTime: Date.now() - 1000, }), // ActionId-keyed entries for pre-submission failure tests 'pre-submission-action-id': { @@ -4682,21 +4975,24 @@ describe('BridgeStatusController', () => { jest.useRealTimers(); }); - describe('TransactionController:transactionFailed', () => { + describe('TransactionController:transactionStatusUpdated (failed)', () => { it('should track failed event for bridge transaction', () => { 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: 'bridgeTxMetaId1', + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.failed, + id: 'bridgeTxMetaId1', + }, }, - }); + ); expect( bridgeStatusController.state.txHistory.bridgeTxMetaId1.status.status, @@ -4721,18 +5017,21 @@ describe('BridgeStatusController', () => { ); 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: abTestsTxMetaId, + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.failed, + id: abTestsTxMetaId, + }, }, - }); + ); expect(messengerCallSpy).toHaveBeenCalledWith( 'BridgeController:trackUnifiedSwapBridgeEvent', @@ -4748,20 +5047,39 @@ describe('BridgeStatusController', () => { it('should track failed event for bridge transaction if approval is dropped', () => { 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.bridgeApproval, - status: TransactionStatus.dropped, - id: 'bridgeApprovalTxMetaId1', + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridgeApproval, + status: TransactionStatus.dropped, + id: 'bridgeApprovalTxMetaId1', + }, }, - }); + ); - expect(messengerCallSpy.mock.lastCall).toMatchSnapshot(); + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.dropped, + id: 'bridgeTxMetaId1WithApproval', + }, + }, + ); + + expect(messengerCallSpy.mock.calls).toMatchSnapshot(); expect( bridgeStatusController.state.txHistory.bridgeTxMetaId1WithApproval .status.status, @@ -4770,18 +5088,21 @@ describe('BridgeStatusController', () => { it('should not track failed event for bridge transaction with featureId', () => { 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: 'perpsBridgeTxMetaId1', + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.failed, + id: 'perpsBridgeTxMetaId1', + }, }, - }); + ); expect( bridgeStatusController.state.txHistory.perpsBridgeTxMetaId1.status @@ -4792,41 +5113,55 @@ describe('BridgeStatusController', () => { it('should track failed event for swap transaction if approval fails', () => { 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.swapApproval, - status: TransactionStatus.failed, - id: 'bridgeApprovalTxMetaId1', + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'approval-tx-error' }, + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.swapApproval, + status: TransactionStatus.failed, + id: 'bridgeApprovalTxMetaId1', + }, }, - }); + ); expect(messengerCallSpy.mock.lastCall).toMatchSnapshot(); expect( bridgeStatusController.state.txHistory.bridgeTxMetaId1WithApproval .status.status, ).toBe(StatusTypes.FAILED); + expect( + bridgeStatusController.state.txHistory.bridgeTxMetaId1WithApproval + .status.srcChain.txHash, + ).toBe('0xsrcTxHash1'); + expect( + bridgeStatusController.state.txHistory.bridgeTxMetaId1WithApproval + .approvalTxId, + ).toBe('bridgeApprovalTxMetaId1'); }); it('should track failed event for bridge transaction if not in txHistory', () => { const messengerCallSpy = jest.spyOn(mockBridgeStatusMessenger, 'call'); const expectedHistory = bridgeStatusController.state.txHistory; - 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: 'bridgeTxMetaIda', + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.failed, + id: 'bridgeTxMetaIda', + }, }, - }); + ); expect(bridgeStatusController.state.txHistory).toStrictEqual( expectedHistory, @@ -4836,18 +5171,21 @@ describe('BridgeStatusController', () => { it('should track failed event for swap transaction', () => { 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.swap, - status: TransactionStatus.failed, - id: 'swapTxMetaId1', + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.swap, + status: TransactionStatus.failed, + id: 'swapTxMetaId1', + }, }, - }); + ); expect( bridgeStatusController.state.txHistory.swapTxMetaId1.status.status, @@ -4857,18 +5195,21 @@ describe('BridgeStatusController', () => { it('should not call getAccountByAddress with undefined when txParams.from is missing', () => { 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: 'bridgeTxMetaId1', + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.failed, + id: 'bridgeTxMetaId1', + }, }, - }); + ); const accountLookupCalls = messengerCallSpy.mock.calls.filter( (call) => call[0] === 'AccountsController:getAccountByAddress', @@ -4881,18 +5222,21 @@ describe('BridgeStatusController', () => { it('should call getAccountByAddress when txParams.from is set', () => { 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: { from: '0xaccount1' } as unknown as TransactionParams, - type: TransactionType.bridge, - status: TransactionStatus.failed, - id: 'bridgeTxMetaId1', + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: { from: '0xaccount1' } as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.failed, + id: 'bridgeTxMetaId1', + }, }, - }); + ); expect(messengerCallSpy).toHaveBeenCalledWith( 'AccountsController:getAccountByAddress', @@ -4902,54 +5246,63 @@ describe('BridgeStatusController', () => { it('should not track failed event for signed status', () => { 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.swap, - status: TransactionStatus.signed, - id: 'swapTxMetaId1', + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.swap, + status: TransactionStatus.signed, + id: 'swapTxMetaId1', + }, }, - }); + ); expect(messengerCallSpy.mock.calls).toMatchSnapshot(); }); it('should not track failed event for approved status', () => { 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.swap, - status: TransactionStatus.approved, - id: 'swapTxMetaId1', + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.swap, + status: TransactionStatus.approved, + id: 'swapTxMetaId1', + }, }, - }); + ); expect(messengerCallSpy.mock.calls).toMatchSnapshot(); }); it('should not track failed event for other transaction types', () => { 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.simpleSend, - status: TransactionStatus.failed, - id: 'simpleSendTxMetaId1', + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.simpleSend, + status: TransactionStatus.failed, + id: 'simpleSendTxMetaId1', + }, }, - }); + ); expect(messengerCallSpy.mock.calls).toMatchSnapshot(); }); @@ -4960,19 +5313,22 @@ describe('BridgeStatusController', () => { const unknownTxMetaId = 'unknown-tx-meta-id'; 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: unknownTxMetaId, - actionId, // ActionId matches the history entry + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + 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( @@ -4987,19 +5343,22 @@ describe('BridgeStatusController', () => { 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, + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'tx-error' }, + 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(); @@ -5014,19 +5373,22 @@ describe('BridgeStatusController', () => { 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, + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + error: { name: 'Error', message: 'User rejected' }, + 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( @@ -5038,7 +5400,7 @@ describe('BridgeStatusController', () => { }); }); - describe('TransactionController:transactionConfirmed', () => { + describe('TransactionController:transactionStatusUpdated (confirmed)', () => { beforeEach(() => { jest.clearAllMocks(); }); @@ -5055,21 +5417,26 @@ describe('BridgeStatusController', () => { 'BridgeStatusController:getBridgeHistoryItemByTxMetaId', 'bridgeTxMetaId1', ); - mockMessenger.publish('TransactionController:transactionConfirmed', { - chainId: CHAIN_IDS.ARBITRUM, - networkClientId: 'eth-id', - time: Date.now(), - txParams: {} as unknown as TransactionParams, - type: TransactionType.bridge, - status: TransactionStatus.confirmed, - id: 'bridgeTxMetaId1', - }); + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.confirmed, + id: 'bridgeTxMetaId1', + }, + }, + ); jest.advanceTimersByTime(500); bridgeStatusController.stopAllPolling(); await flushPromises(); - expect(messengerCallSpy.mock.lastCall).toMatchSnapshot(); + expect(messengerCallSpy.mock.calls).toMatchSnapshot(); expect(mockFetchFn).toHaveBeenCalledTimes(3); expect(mockFetchFn).toHaveBeenCalledWith( 'https://bridge.api.cx.metamask.io/getTxStatus?bridgeId=lifi&srcTxHash=0xsrcTxHash1&bridge=across&srcChainId=42161&destChainId=10&refuel=false&requestId=197c402f-cb96-4096-9f8c-54aed84ca776', @@ -5098,15 +5465,20 @@ describe('BridgeStatusController', () => { mockFetchFn.mockResolvedValueOnce( MockStatusResponse.getComplete({ srcTxHash: '0xperpsSrcTxHash1' }), ); - mockMessenger.publish('TransactionController:transactionConfirmed', { - chainId: CHAIN_IDS.ARBITRUM, - networkClientId: 'eth-id', - time: Date.now(), - txParams: {} as unknown as TransactionParams, - type: TransactionType.bridge, - status: TransactionStatus.confirmed, - id: 'perpsBridgeTxMetaId1', - }); + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.confirmed, + id: 'perpsBridgeTxMetaId1', + }, + }, + ); jest.advanceTimersByTime(30500); bridgeStatusController.stopAllPolling(); @@ -5144,15 +5516,20 @@ describe('BridgeStatusController', () => { mockFetchFn.mockResolvedValueOnce( MockStatusResponse.getFailed({ srcTxHash: '0xperpsSrcTxHash1' }), ); - mockMessenger.publish('TransactionController:transactionConfirmed', { - chainId: CHAIN_IDS.ARBITRUM, - networkClientId: 'eth-id', - time: Date.now(), - txParams: {} as unknown as TransactionParams, - type: TransactionType.bridge, - status: TransactionStatus.confirmed, - id: 'perpsBridgeTxMetaId1', - }); + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.confirmed, + id: 'perpsBridgeTxMetaId1', + }, + }, + ); jest.advanceTimersByTime(40500); bridgeStatusController.stopAllPolling(); @@ -5185,60 +5562,106 @@ describe('BridgeStatusController', () => { it('should track completed event for swap transaction', () => { const messengerCallSpy = jest.spyOn(mockBridgeStatusMessenger, 'call'); - mockMessenger.publish('TransactionController:transactionConfirmed', { - chainId: CHAIN_IDS.ARBITRUM, - networkClientId: 'eth-id', - time: Date.now(), - txParams: {} as unknown as TransactionParams, - type: TransactionType.swap, - status: TransactionStatus.confirmed, - id: 'swapTxMetaId1', - }); + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.swap, + status: TransactionStatus.confirmed, + id: 'swapTxMetaId1', + }, + }, + ); expect(messengerCallSpy.mock.calls).toMatchSnapshot(); }); it('should not track completed event for swap transaction with featureId', () => { const messengerCallSpy = jest.spyOn(mockBridgeStatusMessenger, 'call'); - mockMessenger.publish('TransactionController:transactionConfirmed', { - chainId: CHAIN_IDS.ARBITRUM, - networkClientId: 'eth-id', - time: Date.now(), - txParams: {} as unknown as TransactionParams, - type: TransactionType.swap, - status: TransactionStatus.confirmed, - id: 'perpsSwapTxMetaId1', - }); + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.swap, + status: TransactionStatus.confirmed, + id: 'perpsSwapTxMetaId1', + }, + }, + ); expect(messengerCallSpy).not.toHaveBeenCalled(); }); it('should not track completed event for other transaction types', () => { const messengerCallSpy = jest.spyOn(mockBridgeStatusMessenger, 'call'); - mockMessenger.publish('TransactionController:transactionConfirmed', { - chainId: CHAIN_IDS.ARBITRUM, - networkClientId: 'eth-id', - time: Date.now(), - txParams: {} as unknown as TransactionParams, - type: TransactionType.bridge, - status: TransactionStatus.confirmed, - id: 'bridgeTxMetaId1', - }); + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.confirmed, + id: 'bridgeTxMetaId1', + }, + }, + ); expect(messengerCallSpy.mock.calls).toMatchSnapshot(); }); + it('should not start poll or track completed event if the transaction is an approval', () => { + const messengerCallSpy = jest.spyOn(mockBridgeStatusMessenger, 'call'); + const startPollingSpy = jest.spyOn( + bridgeStatusController, + 'startPolling', + ); + + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.contractInteraction, + status: TransactionStatus.confirmed, + id: 'bridgeApprovalTxMetaId1', + }, + }, + ); + + expect(messengerCallSpy.mock.calls).toHaveLength(0); + expect(startPollingSpy).not.toHaveBeenCalled(); + }); + it('should not start polling for bridge tx if tx is not in txHistory', () => { const messengerCallSpy = jest.spyOn(mockBridgeStatusMessenger, 'call'); - mockMessenger.publish('TransactionController:transactionConfirmed', { - chainId: CHAIN_IDS.ARBITRUM, - networkClientId: 'eth-id', - time: Date.now(), - txParams: {} as unknown as TransactionParams, - type: TransactionType.bridge, - status: TransactionStatus.confirmed, - id: 'bridgeTxMetaId1Unknown', - }); + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: Date.now(), + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.confirmed, + id: 'bridgeTxMetaId1Unknown', + }, + }, + ); expect(messengerCallSpy.mock.calls).toMatchSnapshot(); }); @@ -5250,24 +5673,55 @@ describe('BridgeStatusController', () => { .mockImplementationOnce(jest.fn()); consoleFnSpy.mockImplementationOnce(jest.fn()); - messengerCallSpy.mockImplementation(() => { + messengerCallSpy.mockReturnValueOnce({ + remoteFeatureFlags: { + bridgeConfig: { + maxPendingHistoryItemAgeMs: + DEFAULT_MAX_PENDING_HISTORY_ITEM_AGE_MS, + }, + }, + cacheTimestamp: Date.now(), + }); + + messengerCallSpy.mockImplementationOnce(() => { + throw new Error( + 'AuthenticationController:getBearerToken not implemented', + ); + }); + + messengerCallSpy.mockReturnValueOnce({ + remoteFeatureFlags: { + bridgeConfig: { + maxPendingHistoryItemAgeMs: + DEFAULT_MAX_PENDING_HISTORY_ITEM_AGE_MS, + }, + }, + cacheTimestamp: Date.now(), + }); + messengerCallSpy.mockImplementationOnce(() => { throw new Error( 'AuthenticationController:getBearerToken not implemented', ); }); + mockFetchFn.mockClear(); mockFetchFn.mockResolvedValueOnce( MockStatusResponse.getComplete({ srcTxHash: '0xperpsSrcTxHash1' }), ); - mockMessenger.publish('TransactionController:transactionConfirmed', { - chainId: CHAIN_IDS.ARBITRUM, - networkClientId: 'eth-id', - time: Date.now(), - txParams: {} as unknown as TransactionParams, - type: TransactionType.bridge, - status: TransactionStatus.confirmed, - id: 'perpsBridgeTxMetaId1', - }); + mockMessenger.publish( + 'TransactionController:transactionStatusUpdated', + { + transactionMeta: { + chainId: CHAIN_IDS.ARBITRUM, + networkClientId: 'eth-id', + time: 1729964825189, + txParams: {} as unknown as TransactionParams, + type: TransactionType.bridge, + status: TransactionStatus.confirmed, + id: 'perpsBridgeTxMetaId1', + }, + }, + ); jest.advanceTimersByTime(30500); bridgeStatusController.stopAllPolling(); @@ -5281,6 +5735,13 @@ describe('BridgeStatusController', () => { [ "AuthenticationController:getBearerToken", ], + [ + "AccountsController:getAccountByAddress", + "0xaccount1", + ], + [ + "TransactionController:getState", + ], ] `); expect(mockFetchFn).toHaveBeenCalledWith( @@ -5295,10 +5756,6 @@ describe('BridgeStatusController', () => { "Error getting JWT token for bridge-api request", [Error: AuthenticationController:getBearerToken not implemented], ], - [ - "Error getting JWT token for bridge-api request", - [Error: AuthenticationController:getBearerToken not implemented], - ], ] `); }); diff --git a/packages/bridge-status-controller/src/bridge-status-controller.ts b/packages/bridge-status-controller/src/bridge-status-controller.ts index 03114c30e12..dba05d90512 100644 --- a/packages/bridge-status-controller/src/bridge-status-controller.ts +++ b/packages/bridge-status-controller/src/bridge-status-controller.ts @@ -57,11 +57,16 @@ import { fetchBridgeTxStatus, getStatusRequestWithSrcTxHash, shouldSkipFetchDueToFetchFailures, + shouldWaitForFinalBridgeStatus, } from './utils/bridge-status'; import { getInitialHistoryItem, + getMatchingHistoryEntryForTxMeta, rekeyHistoryItemInState, shouldPollHistoryItem, + incrementPollingAttempts, + isHistoryItemTooOld, + getMatchingHistoryEntryForApprovalTxMeta, } from './utils/history'; import { getIntentFromQuote, @@ -79,6 +84,7 @@ import { getEVMTxPropertiesFromTransactionMeta, getTxStatusesFromHistory, getPreConfirmationPropertiesFromQuote, + getPollingStatusUpdatedProperties, } from './utils/metrics'; import { getNetworkClientIdByChainId, @@ -98,6 +104,7 @@ import { getTransactions, submitEvmTransaction, checkIsDelegatedAccount, + isCrossChainTx, } from './utils/transaction'; const metadata: StateMetadata = { @@ -203,25 +210,65 @@ export class BridgeStatusController extends StaticIntervalPollingController( + 'TransactionController:transactionStatusUpdated', + ({ txMeta, historyKey, historyItem, isApprovalTxMeta }) => { + if (!txMeta) { + return; + } + const { type, status } = txMeta; - this.messenger.subscribe( - 'TransactionController:transactionConfirmed', - (transactionMeta) => { - const { type, id: txMetaId, chainId } = transactionMeta; - if (type === TransactionType.swap) { - this.#trackUnifiedSwapBridgeEvent( - UnifiedSwapBridgeEventName.Completed, - txMetaId, - ); + // Allow event publishing if the txMeta is a swap/bridge OR if the + // corresponding history item exists + const isSwapOrBridgeTransaction = type && isCrossChainTx(type); + if (!isSwapOrBridgeTransaction && !historyKey && !historyItem) { + return; } - if (type === TransactionType.bridge && !isNonEvmChainId(chainId)) { - this.#startPollingForTxId(txMetaId); + + switch (status) { + case TransactionStatus.confirmed: + this.#onTransactionConfirmed({ + txMeta, + historyKey, + isApprovalTxMeta, + }); + break; + case TransactionStatus.failed: + case TransactionStatus.dropped: + case TransactionStatus.rejected: + this.#onTransactionFailed({ txMeta, historyKey, isApprovalTxMeta }); + break; + default: + break; } }, + ({ transactionMeta }) => { + const entry = getMatchingHistoryEntryForTxMeta( + this.state.txHistory, + transactionMeta, + ); + const approvalEntry = getMatchingHistoryEntryForApprovalTxMeta( + this.state.txHistory, + transactionMeta, + ); + const entryToUse = entry ?? approvalEntry; + + return { + historyKey: entryToUse?.[0], + historyItem: entryToUse?.[1], + txMeta: transactionMeta, + isApprovalTxMeta: + entryToUse?.[1]?.approvalTxId === transactionMeta.id, + }; + }, ); // If you close the extension, but keep the browser open, the polling continues @@ -231,87 +278,85 @@ export class BridgeStatusController extends StaticIntervalPollingController { - const { type, status, id: txMetaId, actionId } = transactionMeta; - - if ( - type && - [ - TransactionType.bridge, - TransactionType.swap, - TransactionType.bridgeApproval, - TransactionType.swapApproval, - ].includes(type) && - [ - TransactionStatus.failed, - TransactionStatus.dropped, - TransactionStatus.rejected, - ].includes(status) - ) { - this.#markTxAsFailed(transactionMeta); - if (status !== TransactionStatus.rejected) { - let historyKey: string | undefined; - if (this.state.txHistory[txMetaId]) { - historyKey = txMetaId; - } else if (actionId && this.state.txHistory[actionId]) { - historyKey = actionId; - } - - const activeHistoryKey = historyKey ?? txMetaId; - - // Skip account lookup and tracking when featureId is set (e.g. PERPS) - if (this.state.txHistory[activeHistoryKey]?.featureId) { - return; - } + // Check if the history item is already marked as a failure + const isHistoryItemAlreadyFailed = historyKey + ? this.state.txHistory[historyKey]?.status.status === StatusTypes.FAILED + : false; + + this.#updateHistoryItem({ + historyKey, + status: StatusTypes.FAILED, + txHash: isApprovalTxMeta ? undefined : txMeta.hash, + }); - const from = transactionMeta.txParams?.from; - this.#trackUnifiedSwapBridgeEvent( - UnifiedSwapBridgeEventName.Failed, - activeHistoryKey, - getEVMTxPropertiesFromTransactionMeta( - transactionMeta, - from - ? this.messenger.call( - 'AccountsController:getAccountByAddress', - from, - ) - : undefined, - ), - ); - } + if (txMeta.status === TransactionStatus.rejected) { + return; } - }; - // Mark tx as failed in txHistory if either the approval or trade fails - readonly #markTxAsFailed = ({ - id: txMetaId, - actionId, - }: TransactionMeta): void => { - // Look up by txMetaId first - let txHistoryKey: string | undefined = this.state.txHistory[txMetaId] - ? txMetaId - : undefined; + // Skip account lookup and tracking when featureId is set (e.g. PERPS) + if (historyKey && this.state.txHistory[historyKey]?.featureId) { + return; + } - // If not found by txMetaId, try looking up by actionId (for pre-submission failures) - if (!txHistoryKey && actionId && this.state.txHistory[actionId]) { - txHistoryKey = actionId; + // Skip tracking if this is a duplicate failed event for the same history item + // This can happen if the transaction includes an approval tx that fails + if (isHistoryItemAlreadyFailed) { + return; } - // If still not found, try looking up by approvalTxId - txHistoryKey ??= Object.keys(this.state.txHistory).find( - (key) => this.state.txHistory[key].approvalTxId === txMetaId, + this.#trackUnifiedSwapBridgeEvent( + UnifiedSwapBridgeEventName.Failed, + historyKey, + getEVMTxPropertiesFromTransactionMeta(txMeta), ); + }; - if (!txHistoryKey) { + // Only EVM txs + readonly #onTransactionConfirmed = ({ + txMeta, + historyKey, + isApprovalTxMeta, + }: { + txMeta: TransactionMeta; + historyKey?: string; + isApprovalTxMeta: boolean; + }): void => { + // Return early if the confirmed txMeta is for an approval since we + // still need to wait for the trade to be confirmed + if (isApprovalTxMeta) { return; } - const key = txHistoryKey; - this.update((statusState) => { - statusState.txHistory[key].status.status = StatusTypes.FAILED; + + this.#updateHistoryItem({ + historyKey, + txHash: txMeta.hash, }); + + switch (txMeta.type) { + case TransactionType.swap: + this.#updateHistoryItem({ + historyKey, + status: StatusTypes.COMPLETE, + }); + this.#trackUnifiedSwapBridgeEvent( + UnifiedSwapBridgeEventName.Completed, + historyKey, + ); + break; + default: + if (historyKey) { + this.#startPollingForTxId(historyKey); + } + break; + } }; resetState = (): void => { @@ -456,54 +501,53 @@ export class BridgeStatusController extends StaticIntervalPollingController { // Check for historyItems that do not have a status of complete and restart polling const { txHistory } = this.state; - const historyItems = Object.values(txHistory); + const historyItems = Object.entries(txHistory); const incompleteHistoryItems = historyItems .filter( - (historyItem) => + ([_, historyItem]) => historyItem.status.status === StatusTypes.PENDING || historyItem.status.status === StatusTypes.UNKNOWN, ) // Only poll items with txMetaId (post-submission items) - .filter( - ( - historyItem, - ): historyItem is BridgeHistoryItem & { txMetaId: string } => - Boolean(historyItem.txMetaId), - ) - .filter((historyItem) => { + .filter(([_, historyItem]: [string, BridgeHistoryItem]) => { + if (!historyItem.txMetaId) { + return false; + } // Check if we are already polling this tx, if so, skip restarting polling for that const pollingToken = this.#pollingTokensByTxMetaId[historyItem.txMetaId]; return !pollingToken; }) // Only restart polling for items that still require status updates - .filter((historyItem) => { + .filter(([_, historyItem]: [string, BridgeHistoryItem]) => { return shouldPollHistoryItem(historyItem); }); - incompleteHistoryItems.forEach((historyItem) => { - const bridgeTxMetaId = historyItem.txMetaId; - const shouldSkipFetch = shouldSkipFetchDueToFetchFailures( - historyItem.attempts, - ); - if (shouldSkipFetch) { - return; - } + incompleteHistoryItems.forEach( + ([historyKey, historyItem]: [string, BridgeHistoryItem]) => { + const shouldSkipFetch = shouldSkipFetchDueToFetchFailures( + historyItem.attempts, + ); + if (shouldSkipFetch) { + return; + } - // We manually call startPolling() here rather than go through startPollingForBridgeTxStatus() - // because we don't want to overwrite the existing historyItem in state - this.#startPollingForTxId(bridgeTxMetaId); - }); + // We manually call startPolling() here rather than go through startPollingForBridgeTxStatus() + // because we don't want to overwrite the existing historyItem in state + this.#startPollingForTxId(historyKey); + }, + ); }; readonly #addTxToHistory = ( ...args: Parameters - ): void => { + ): string => { const { historyKey, txHistoryItem } = getInitialHistoryItem(...args); this.update((state) => { // Use actionId as key for pre-submission, or txMeta.id for post-submission state.txHistory[historyKey] = txHistoryItem; }); + return historyKey; }; /** @@ -532,10 +576,7 @@ export class BridgeStatusController extends StaticIntervalPollingController { - const { attempts } = this.state.txHistory[bridgeTxMetaId]; + readonly #handleFetchFailure = async ( + bridgeTxMetaId: string, + pollingStatus: PollingStatus, + ): Promise => { + // Increment the polling attempts counter + const newAttempts = incrementPollingAttempts( + this.state.txHistory[bridgeTxMetaId], + ); + this.#updateHistoryItem({ + historyKey: bridgeTxMetaId, + attempts: newAttempts, + }); + // Continue polling every 10s until the max attempts is reached + if (newAttempts.counter < MAX_ATTEMPTS) { + return; + } + const pollingToken = this.#pollingTokensByTxMetaId[bridgeTxMetaId]; - const newAttempts = attempts - ? { - counter: attempts.counter + 1, - lastAttemptTime: Date.now(), - } - : { - counter: 1, - lastAttemptTime: Date.now(), - }; + // Track polling status updated event + this.#trackPollingStatusUpdatedEvent(bridgeTxMetaId, pollingStatus); - // If we've failed too many times, stop polling for the tx - const pollingToken = this.#pollingTokensByTxMetaId[bridgeTxMetaId]; - if (newAttempts.counter >= MAX_ATTEMPTS && pollingToken) { + // After the max attempts, keep polling with exponential backoff + // If the historyItem age is less than the configured maxPendingHistoryItemAgeMs flag, wait for a valid srcTxHash + if ( + await shouldWaitForFinalBridgeStatus( + this.messenger, + this.state.txHistory[bridgeTxMetaId], + ) + ) { + return; + } + + // If we've failed too many times and the srcTxHash is invalid, stop polling for the tx + if (pollingToken) { this.stopPollingByPollingToken(pollingToken); delete this.#pollingTokensByTxMetaId[bridgeTxMetaId]; - - // Track max polling reached event - const historyItem = this.state.txHistory[bridgeTxMetaId]; - if (historyItem && !historyItem.featureId) { - const selectedAccount = getAccountByAddress( - this.messenger, - historyItem.account, - ); - const requestParams = getRequestParamFromHistory(historyItem); - const requestMetadata = getRequestMetadataFromHistory( - historyItem, - selectedAccount, - ); - const { security_warnings: _, ...metadataWithoutWarnings } = - requestMetadata; - - this.#trackUnifiedSwapBridgeEvent( - UnifiedSwapBridgeEventName.PollingStatusUpdated, - bridgeTxMetaId, - { - ...getTradeDataFromHistory(historyItem), - ...getPriceImpactFromQuote(historyItem.quote), - ...metadataWithoutWarnings, - chain_id_source: requestParams.chain_id_source, - chain_id_destination: requestParams.chain_id_destination, - token_symbol_source: requestParams.token_symbol_source, - token_symbol_destination: requestParams.token_symbol_destination, - action_type: MetricsActionType.SWAPBRIDGE_V1, - polling_status: PollingStatus.MaxPollingReached, - retry_attempts: newAttempts.counter, - }, - ); - } } - // Update the attempts counter - this.update((state) => { - state.txHistory[bridgeTxMetaId].attempts = newAttempts; - }); + if (pollingStatus === PollingStatus.InvalidTransactionHash) { + // Delete the history item so polling doesn't start over on the next restart + this.#deleteHistoryItem(bridgeTxMetaId); + } }; readonly #fetchBridgeTxStatus = async ({ @@ -662,6 +689,7 @@ export class BridgeStatusController extends StaticIntervalPollingController { + /** + * Returns the srcTxHash for a non-STX EVM tx, the hash from the bridge status api, + * or the local hash from the TransactionController if the tx is in a finalized state + * + * @param bridgeTxMetaId - The bridge tx meta id + * @returns The srcTxHash + */ + readonly #setAndGetSrcTxHash = ( + bridgeTxMetaId: string, + ): string | undefined => { const { txHistory } = this.state; - // Prefer the srcTxHash from bridgeStatusState so we don't have to l ook up in TransactionController + // Prefer the srcTxHash from bridgeStatusState so we don't have to look up in TransactionController // But it is possible to have bridgeHistoryItem in state without the srcTxHash yet when it is an STX const srcTxHash = txHistory[bridgeTxMetaId].status.srcChain.txHash; - if (srcTxHash) { + if ( + srcTxHash || + isNonEvmChainId(txHistory[bridgeTxMetaId].quote.srcChainId) + ) { return srcTxHash; } - // Look up in TransactionController if txMeta has been updated with the srcTxHash + // Update history with TransactionController's hash if it has been updated const txMeta = getTransactionMetaById(this.messenger, bridgeTxMetaId); - return txMeta?.hash; + + if (!txMeta) { + return undefined; + } + + // Wait for finalized status before updating the history item + const localTxHash = [ + TransactionStatus.confirmed, + TransactionStatus.dropped, + TransactionStatus.rejected, + TransactionStatus.failed, + ].includes(txMeta.status) + ? txMeta.hash + : undefined; + this.#updateHistoryItem({ + historyKey: bridgeTxMetaId, + txHash: localTxHash, + }); + + return localTxHash; }; - readonly #updateSrcTxHash = ( - bridgeTxMetaId: string, - srcTxHash: string, - ): void => { - const { txHistory } = this.state; - if (txHistory[bridgeTxMetaId].status.srcChain.txHash) { + readonly #updateHistoryItem = ({ + historyKey, + status, + txHash, + attempts, + }: { + historyKey?: string; + status?: StatusTypes; + txHash?: string; + attempts?: BridgeHistoryItem['attempts']; + }): void => { + if (!historyKey) { return; } + this.update((currentState) => { + if (status) { + currentState.txHistory[historyKey].status.status = status; + } + if (txHash) { + currentState.txHistory[historyKey].status.srcChain.txHash = txHash; + } + if (attempts) { + currentState.txHistory[historyKey].attempts = attempts; + } + }); + }; - this.update((state) => { - state.txHistory[bridgeTxMetaId].status.srcChain.txHash = srcTxHash; + readonly #deleteHistoryItem = (historyKey: string): void => { + this.update((currentState) => { + delete currentState.txHistory[historyKey]; }); }; @@ -848,6 +935,7 @@ export class BridgeStatusController extends StaticIntervalPollingController { + // Track polling status updated event + const historyItem = this.state.txHistory[historyKey]; + if (historyItem && !historyItem.featureId) { + this.#trackUnifiedSwapBridgeEvent( + UnifiedSwapBridgeEventName.PollingStatusUpdated, + historyKey, + getPollingStatusUpdatedProperties( + this.messenger, + pollingStatus, + historyItem, + ), + ); + } + }; + /** * Tracks post-submission events for a cross-chain swap based on the history item * diff --git a/packages/bridge-status-controller/src/constants.ts b/packages/bridge-status-controller/src/constants.ts index ba9251f1e33..7c98fe3339c 100644 --- a/packages/bridge-status-controller/src/constants.ts +++ b/packages/bridge-status-controller/src/constants.ts @@ -2,6 +2,7 @@ import type { BridgeStatusControllerState } from './types'; export const REFRESH_INTERVAL_MS = 10 * 1000; // 10 seconds export const MAX_ATTEMPTS = 7; // at 7 attempts, delay is 10:40, cumulative time is 21:10 +export const DEFAULT_MAX_PENDING_HISTORY_ITEM_AGE_MS = 2 * 24 * 60 * 60 * 1000; // 2 days export const BRIDGE_STATUS_CONTROLLER_NAME = 'BridgeStatusController'; diff --git a/packages/bridge-status-controller/src/types.ts b/packages/bridge-status-controller/src/types.ts index 20ba3f21619..0233d83535e 100644 --- a/packages/bridge-status-controller/src/types.ts +++ b/packages/bridge-status-controller/src/types.ts @@ -22,6 +22,7 @@ import type { NetworkControllerGetStateAction, } from '@metamask/network-controller'; import type { AuthenticationControllerGetBearerTokenAction } from '@metamask/profile-sync-controller/auth'; +import type { RemoteFeatureFlagControllerGetStateAction } from '@metamask/remote-feature-flag-controller'; import type { SnapControllerHandleRequestAction } from '@metamask/snaps-controllers'; import type { Infer } from '@metamask/superstruct'; import type { @@ -29,8 +30,7 @@ import type { TransactionControllerEstimateGasFeeAction, TransactionControllerGetStateAction, TransactionControllerIsAtomicBatchSupportedAction, - TransactionControllerTransactionConfirmedEvent, - TransactionControllerTransactionFailedEvent, + TransactionControllerTransactionStatusUpdatedEvent, TransactionControllerUpdateTransactionAction, TransactionMeta, } from '@metamask/transaction-controller'; @@ -116,7 +116,7 @@ export type BridgeHistoryItem = { batchId?: string; quote: Quote; status: StatusResponse; - startTime?: number; // timestamp in ms + startTime: number; // timestamp in ms estimatedProcessingTimeInSeconds: number; slippagePercentage: number; completionTime?: number; // timestamp in ms @@ -225,7 +225,7 @@ export type StartPollingForBridgeTxStatusArgs = { */ originalTransactionId?: string; quoteResponse: QuoteResponse & QuoteMetadata; - startTime?: BridgeHistoryItem['startTime']; + startTime: BridgeHistoryItem['startTime']; slippagePercentage: BridgeHistoryItem['slippagePercentage']; initialDestAssetBalance?: BridgeHistoryItem['initialDestAssetBalance']; targetContractAddress?: BridgeHistoryItem['targetContractAddress']; @@ -292,6 +292,7 @@ type AllowedActions = | NetworkControllerFindNetworkClientIdByChainIdAction | NetworkControllerGetStateAction | NetworkControllerGetNetworkClientByIdAction + | RemoteFeatureFlagControllerGetStateAction | SnapControllerHandleRequestAction | TransactionControllerGetStateAction | TransactionControllerUpdateTransactionAction @@ -308,9 +309,7 @@ type AllowedActions = /** * The external events available to the BridgeStatusController. */ -type AllowedEvents = - | TransactionControllerTransactionFailedEvent - | TransactionControllerTransactionConfirmedEvent; +type AllowedEvents = TransactionControllerTransactionStatusUpdatedEvent; /** * The messenger for the BridgeStatusController. diff --git a/packages/bridge-status-controller/src/utils/bridge-status.ts b/packages/bridge-status-controller/src/utils/bridge-status.ts index 0710843e8ee..0f4677057c3 100644 --- a/packages/bridge-status-controller/src/utils/bridge-status.ts +++ b/packages/bridge-status-controller/src/utils/bridge-status.ts @@ -1,4 +1,4 @@ -import { getClientHeaders } from '@metamask/bridge-controller'; +import { getClientHeaders, isNonEvmChainId } from '@metamask/bridge-controller'; import type { Quote, QuoteResponse } from '@metamask/bridge-controller'; import { StructError } from '@metamask/superstruct'; @@ -10,7 +10,9 @@ import type { FetchFunction, BridgeHistoryItem, StatusRequest, + BridgeStatusControllerMessenger, } from '../types'; +import { getNetworkClientByChainId } from './network'; import { validateBridgeStatusResponse } from './validators'; export const getBridgeStatusUrl = (bridgeApiBaseUrl: string): string => @@ -112,6 +114,52 @@ export const shouldSkipFetchDueToFetchFailures = ( return false; }; +/* + * Checks if a pending history item is older than 2 days and does not have a valid tx hash + * + * @param messenger - The messenger to use to get the transaction meta by hash or id + * @param historyItem - The history item to check + * + * @returns true if the src tx hash is valid or we should still wait for it, false otherwise + */ +export const shouldWaitForFinalBridgeStatus = async ( + messenger: BridgeStatusControllerMessenger, + historyItem: BridgeHistoryItem, +): Promise => { + if (isNonEvmChainId(historyItem.quote.srcChainId)) { + return true; + } + + // Otherwise check if the tx has been mined on chain + const provider = getNetworkClientByChainId( + messenger, + historyItem.quote.srcChainId, + ); + // When this happens it means the network was disabled while the tx was pending + if (!provider) { + return false; + } + + if (!historyItem.status.srcChain.txHash) { + return false; + } + + return provider + .request({ + method: 'eth_getTransactionReceipt', + params: [historyItem.status.srcChain.txHash], + }) + .then((txReceipt) => { + if (txReceipt) { + return true; + } + return false; + }) + .catch(() => { + return false; + }); +}; + /** * @deprecated Use getStatusRequestWithSrcTxHash instead * @param quoteResponse - The quote response to get the status request parameters from diff --git a/packages/bridge-status-controller/src/utils/feature-flags.ts b/packages/bridge-status-controller/src/utils/feature-flags.ts new file mode 100644 index 00000000000..7194f9cfbe7 --- /dev/null +++ b/packages/bridge-status-controller/src/utils/feature-flags.ts @@ -0,0 +1,14 @@ +import { getBridgeFeatureFlags } from '@metamask/bridge-controller'; + +import { DEFAULT_MAX_PENDING_HISTORY_ITEM_AGE_MS } from '../constants'; +import { BridgeStatusControllerMessenger } from '../types'; + +export const getMaxPendingHistoryItemAgeMs = ( + messenger: BridgeStatusControllerMessenger, +): number => { + const bridgeFeatureFlags = getBridgeFeatureFlags(messenger); + return ( + bridgeFeatureFlags.maxPendingHistoryItemAgeMs ?? + DEFAULT_MAX_PENDING_HISTORY_ITEM_AGE_MS + ); +}; diff --git a/packages/bridge-status-controller/src/utils/history.ts b/packages/bridge-status-controller/src/utils/history.ts index 9a7c1921b79..33c54de3457 100644 --- a/packages/bridge-status-controller/src/utils/history.ts +++ b/packages/bridge-status-controller/src/utils/history.ts @@ -1,14 +1,18 @@ import { StatusTypes, isCrossChain, + isNonEvmChainId, isTronChainId, } from '@metamask/bridge-controller'; +import type { TransactionMeta } from '@metamask/transaction-controller'; import type { BridgeHistoryItem, + BridgeStatusControllerMessenger, BridgeStatusControllerState, StartPollingForBridgeTxStatusArgsSerialized, } from '../types'; +import { getMaxPendingHistoryItemAgeMs } from './feature-flags'; export const rekeyHistoryItemInState = ( state: BridgeStatusControllerState, @@ -36,6 +40,57 @@ export const rekeyHistoryItemInState = ( return true; }; +/** + * Returns the history entry that matches the txMeta by id, actionId, batchId, or txHash + * + * @param txHistory - The transaction history + * @param txMeta - The transaction meta + * @returns The history entry that matches the txMeta + */ +export const getMatchingHistoryEntryForTxMeta = ( + txHistory: BridgeStatusControllerState['txHistory'], + txMeta: TransactionMeta, +): [string, BridgeHistoryItem] | undefined => { + const historyEntries = Object.entries(txHistory); + + return historyEntries.find(([key, value]) => { + const { + txMetaId, + actionId, + batchId, + status: { + srcChain: { txHash }, + }, + } = value; + return ( + key === txMeta.id || + key === txMeta.actionId || + txMetaId === txMeta.id || + (actionId ? actionId === txMeta.actionId : false) || + (batchId ? batchId === txMeta.batchId : false) || + (txHash ? txHash.toLowerCase() === txMeta.hash?.toLowerCase() : false) + ); + }); +}; + +/** + * Returns the history entry whose approvalTxId matches the approval transaction + * + * @param txHistory - The transaction history + * @param txMeta - The transaction meta + * @returns The history entry that matches the txMeta + */ +export const getMatchingHistoryEntryForApprovalTxMeta = ( + txHistory: BridgeStatusControllerState['txHistory'], + txMeta: TransactionMeta, +): [string, BridgeHistoryItem] | undefined => { + const historyEntries = Object.entries(txHistory); + + return historyEntries.find(([_, value]) => + value.approvalTxId ? value.approvalTxId === txMeta.id : false, + ); +}; + /** * Determines the key to use for storing a bridge history item. * Uses actionId for pre-submission tracking, or bridgeTxMetaId for post-submission. @@ -119,7 +174,11 @@ export const getInitialHistoryItem = ( status: StatusTypes.PENDING, srcChain: { chainId: quoteResponse.quote.srcChainId, - txHash: bridgeTxMeta?.hash, + // STX transactions don't have a final tx hash until they are mined, so we don't set it initially + txHash: + isNonEvmChainId(quoteResponse.quote.srcChainId) || !isStxEnabled + ? bridgeTxMeta?.hash + : undefined, }, }, hasApprovalTx: Boolean(quoteResponse.approval), @@ -146,3 +205,28 @@ export const shouldPollHistoryItem = ( return [isBridgeTx, isIntent, isTronTx].some(Boolean); }; + +export const isHistoryItemTooOld = ( + messenger: BridgeStatusControllerMessenger, + historyItem: BridgeHistoryItem, +): boolean => { + const maxPendingHistoryItemAgeMs = getMaxPendingHistoryItemAgeMs(messenger); + + return Date.now() - historyItem.startTime > maxPendingHistoryItemAgeMs; +}; + +export const incrementPollingAttempts = ( + historyItem: BridgeHistoryItem, +): NonNullable => { + const { attempts } = historyItem; + const newAttempts = attempts + ? { + counter: attempts.counter + 1, + lastAttemptTime: Date.now(), + } + : { + counter: 1, + lastAttemptTime: Date.now(), + }; + return newAttempts; +}; diff --git a/packages/bridge-status-controller/src/utils/metrics.test.ts b/packages/bridge-status-controller/src/utils/metrics.test.ts index 5eaf1eb01ae..e276f9318e1 100644 --- a/packages/bridge-status-controller/src/utils/metrics.test.ts +++ b/packages/bridge-status-controller/src/utils/metrics.test.ts @@ -1049,7 +1049,7 @@ describe('metrics utils', () => { it('should return correct properties for a successful swap transaction', () => { const result = getEVMTxPropertiesFromTransactionMeta(mockTransactionMeta); expect(result).toStrictEqual({ - error_message: '', + error_message: 'Transaction submitted', chain_id_source: 'eip155:1', chain_id_destination: 'eip155:1', token_symbol_source: 'ETH', @@ -1086,14 +1086,14 @@ describe('metrics utils', () => { ...mockTransactionMeta, status: TransactionStatus.failed, error: { - message: 'Transaction failed', + message: 'Error message', name: 'Error', } as TransactionError, }; const result = getEVMTxPropertiesFromTransactionMeta( failedTransactionMeta, ); - expect(result.error_message).toBe('Transaction failed'); + expect(result.error_message).toBe('Transaction failed. Error message'); expect(result.source_transaction).toBe('FAILED'); }); diff --git a/packages/bridge-status-controller/src/utils/metrics.ts b/packages/bridge-status-controller/src/utils/metrics.ts index 75611842b87..8f53fc5548b 100644 --- a/packages/bridge-status-controller/src/utils/metrics.ts +++ b/packages/bridge-status-controller/src/utils/metrics.ts @@ -25,6 +25,7 @@ import type { RequestParams, TradeData, RequestMetadata, + PollingStatus, } from '@metamask/bridge-controller'; import { TransactionStatus, @@ -34,7 +35,11 @@ import type { TransactionMeta } from '@metamask/transaction-controller'; import type { CaipAssetType } from '@metamask/utils'; import { BigNumber } from 'bignumber.js'; -import type { BridgeHistoryItem } from '../types'; +import type { + BridgeHistoryItem, + BridgeStatusControllerMessenger, +} from '../types'; +import { getAccountByAddress } from './accounts'; import { calcActualGasUsed } from './gas'; import { getActualBridgeReceivedAmount, @@ -275,7 +280,12 @@ export const getEVMTxPropertiesFromTransactionMeta = ( ].includes(transactionMeta.status) ? StatusTypes.FAILED : StatusTypes.COMPLETE, - error_message: transactionMeta.error?.message ?? '', + error_message: [ + `Transaction ${transactionMeta.status}`, + transactionMeta.error?.message, + ] + .filter(Boolean) + .join('. '), chain_id_source: formatChainIdToCaip(transactionMeta.chainId), chain_id_destination: formatChainIdToCaip(transactionMeta.chainId), token_symbol_source: transactionMeta.sourceTokenSymbol ?? '', @@ -318,3 +328,30 @@ export const getEVMTxPropertiesFromTransactionMeta = ( action_type: MetricsActionType.SWAPBRIDGE_V1, }; }; + +export const getPollingStatusUpdatedProperties = ( + messenger: BridgeStatusControllerMessenger, + pollingStatus: PollingStatus, + historyItem: BridgeHistoryItem, +) => { + const selectedAccount = getAccountByAddress(messenger, historyItem.account); + const requestParams = getRequestParamFromHistory(historyItem); + const requestMetadata = getRequestMetadataFromHistory( + historyItem, + selectedAccount, + ); + const { security_warnings: _, ...metadataWithoutWarnings } = requestMetadata; + + return { + ...getTradeDataFromHistory(historyItem), + ...getPriceImpactFromQuote(historyItem.quote), + ...metadataWithoutWarnings, + chain_id_source: requestParams.chain_id_source, + chain_id_destination: requestParams.chain_id_destination, + token_symbol_source: requestParams.token_symbol_source, + token_symbol_destination: requestParams.token_symbol_destination, + action_type: MetricsActionType.SWAPBRIDGE_V1, + polling_status: pollingStatus, + retry_attempts: historyItem.attempts?.counter ?? 0, + }; +}; diff --git a/packages/bridge-status-controller/src/utils/network.ts b/packages/bridge-status-controller/src/utils/network.ts index 7a81d8e62d9..98bc47f9e1e 100644 --- a/packages/bridge-status-controller/src/utils/network.ts +++ b/packages/bridge-status-controller/src/utils/network.ts @@ -1,10 +1,9 @@ /* eslint-disable @typescript-eslint/explicit-function-return-type */ -import { - formatChainIdToHex, - GenericQuoteRequest, -} from '@metamask/bridge-controller'; +import { formatChainIdToHex } from '@metamask/bridge-controller'; +import type { GenericQuoteRequest } from '@metamask/bridge-controller'; +import type { NetworkClient } from '@metamask/network-controller'; -import { BridgeStatusControllerMessenger } from '../types'; +import type { BridgeStatusControllerMessenger } from '../types'; export const getSelectedChainId = ( messenger: BridgeStatusControllerMessenger, @@ -29,3 +28,16 @@ export const getNetworkClientIdByChainId = ( hexChainId, ); }; + +export const getNetworkClientByChainId = ( + messenger: BridgeStatusControllerMessenger, + chainId: GenericQuoteRequest['srcChainId'], +): NetworkClient['provider'] => { + const networkClientId = getNetworkClientIdByChainId(messenger, chainId); + + const networkClient = messenger.call( + 'NetworkController:getNetworkClientById', + networkClientId, + ); + return networkClient.provider; +}; diff --git a/packages/bridge-status-controller/src/utils/transaction.ts b/packages/bridge-status-controller/src/utils/transaction.ts index 8c94a61bb63..b4a4a39f1c3 100644 --- a/packages/bridge-status-controller/src/utils/transaction.ts +++ b/packages/bridge-status-controller/src/utils/transaction.ts @@ -30,6 +30,14 @@ import type { BridgeStatusControllerMessenger } from '../types'; import { getAccountByAddress } from './accounts'; import { getNetworkClientIdByChainId } from './network'; +const isApprovalTx = (type: TransactionType) => + type === TransactionType.bridgeApproval || + type === TransactionType.swapApproval; +const isTradeTx = (type: TransactionType) => + type === TransactionType.bridge || type === TransactionType.swap; +export const isCrossChainTx = (type: TransactionType) => + isTradeTx(type) || isApprovalTx(type); + export const getGasFeeEstimates = async ( messenger: BridgeStatusControllerMessenger, args: Parameters[0], @@ -145,7 +153,7 @@ export const getTransactionMetaByHash = ( txHash?: string, ) => { return getTransactions(messenger).find( - (tx: TransactionMeta) => tx.hash === txHash, + (tx: TransactionMeta) => tx.hash?.toLowerCase() === txHash?.toLowerCase(), ); }; @@ -501,19 +509,11 @@ export const findAndUpdateTransactionsInBatch = ({ if (is7702Transaction) { // For 7702 transactions, we need to match based on transaction type // since the data field might be different (batch execute call) - if ( - (txType === TransactionType.swap || - txType === TransactionType.bridge) && - tx.type === TransactionType.batch - ) { + if (isTradeTx(txType) && tx.type === TransactionType.batch) { return true; } // Also check if it's an approval transaction for 7702 - if ( - (txType === TransactionType.swapApproval || - txType === TransactionType.bridgeApproval) && - tx.txParams.data === txData - ) { + if (isApprovalTx(txType) && tx.txParams.data === txData) { return true; } }