-
-
Notifications
You must be signed in to change notification settings - Fork 262
feat: CowSwap intent-based swaps #6547
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
4ec292c to
dde0e40
Compare
| return TransactionStatus.failed; | ||
| default: | ||
| return TransactionStatus.submitted; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CANCELLED intent status incorrectly mapped to non-terminal states
The IntentOrderStatus.CANCELLED enum value is defined but not handled in two places. In mapIntentOrderStatusToTransactionStatus, a cancelled order falls through to the default case returning TransactionStatus.submitted. In #updateBridgeHistoryFromIntentOrder, it's not included in any status check so it maps to StatusTypes.UNKNOWN. Both are incorrect since CANCELLED is a terminal state (like EXPIRED) and should be mapped to a failed status. A cancelled order would appear as still in-progress rather than completed unsuccessfully.
Additional Locations (1)
| }; | ||
|
|
||
| resetState = () => { | ||
| resetState = (): void => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
State reset methods don't stop active polling
Both resetState() and wipeBridgeStatus({ ignoreNetwork: true }) clear the txHistory state but don't stop active polling or clean up #pollingTokensByTxMetaId. In contrast, #wipeBridgeStatusByChainId (used when ignoreNetwork: false) properly stops polling for each item. After these methods are called, orphaned polling continues indefinitely, making repeated calls to _executePoll that find no history items and return early. This wastes resources through unnecessary timer activity and potential network requests.
Additional Locations (1)
| return TransactionStatus.failed; | ||
| default: | ||
| return TransactionStatus.submitted; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CANCELLED intent status maps to submitted instead of failed
The IntentOrderStatus.CANCELLED enum value is defined in validators.ts but not explicitly handled in either mapIntentOrderStatusToTransactionStatus or #updateBridgeHistoryFromIntentOrder. In mapIntentOrderStatusToTransactionStatus, a cancelled order falls through to the default case returning TransactionStatus.submitted, while in #updateBridgeHistoryFromIntentOrder it maps to StatusTypes.UNKNOWN. This inconsistency means a user-cancelled intent order would incorrectly appear as "submitted" in the transaction controller but "unknown" in bridge history. A cancelled order should be treated as failed in both locations.
Additional Locations (1)
| address: string; | ||
| ignoreNetwork: boolean; | ||
| }) => { | ||
| }): void => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wipeBridgeStatus ignoreNetwork path causes uncaught crash
When wipeBridgeStatus is called with ignoreNetwork: true, it wipes all history items without stopping active polling. Unlike the ignoreNetwork: false path which calls stopPollingByPollingToken before deletion, this path directly clears txHistory. When the next poll cycle executes, #fetchBridgeTxStatus crashes accessing the deleted item, gets caught by try-catch, and calls #handleFetchFailure. However, #handleFetchFailure also accesses this.state.txHistory[bridgeTxMetaId] at line 569 without null checking, causing an uncaught exception since the item no longer exists.
Additional Locations (1)
4a038f9 to
0f5f640
Compare
| // Check metadata for additional transaction hashes | ||
| const metadataTxHashes = Array.isArray(intentOrder.metadata.txHashes) | ||
| ? intentOrder.metadata.txHashes | ||
| : []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String txHashes value ignored, converted to empty array
The IntentOrder type in validators.ts defines metadata.txHashes as string[] | string, but #updateBridgeHistoryFromIntentOrder only handles the array case. When txHashes is a single string (not wrapped in an array), the code sets metadataTxHashes to an empty array [], causing the transaction hash to be lost. If txHash is also empty, the history item won't receive any hashes. The condition should wrap a single string in an array instead of discarding it.
| statusType = StatusTypes.SUBMITTED; | ||
| } else { | ||
| statusType = StatusTypes.UNKNOWN; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CANCELLED intent status causes infinite polling and inconsistent state
The IntentOrderStatus.CANCELLED enum value is defined but not handled in status mapping logic. In #updateBridgeHistoryFromIntentOrder, cancelled orders fall through to StatusTypes.UNKNOWN, and in mapIntentOrderStatusToTransactionStatus, they map to TransactionStatus.submitted. Since UNKNOWN is not a final state (only COMPLETE and FAILED are checked on line 893-895), polling continues indefinitely for cancelled orders. Additionally, the transaction displays as "submitted" when it will never execute, causing user confusion.
Additional Locations (1)
| status: statusType, | ||
| srcChain: { | ||
| chainId: srcChainId, | ||
| txHash: txHash ?? historyItem.status.srcChain.txHash ?? '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nullish coalescing fails to preserve existing txHash
The txHash variable is initialized to an empty string '' when intentOrder.txHash is undefined (line 786). On line 805, the code uses txHash ?? historyItem.status.srcChain.txHash ?? '' intending to fall back to the existing historyItem.status.srcChain.txHash. However, the nullish coalescing operator ?? only treats null and undefined as nullish — empty string '' is not nullish. So when intentOrder.txHash is undefined, txHash is '', and the expression returns '' instead of preserving the existing historyItem.status.srcChain.txHash. This causes loss of transaction hash data during intent order status updates.
Additional Locations (1)
| const isFailed = [ | ||
| IntentOrderStatus.FAILED, | ||
| IntentOrderStatus.EXPIRED, | ||
| ].includes(intentOrder.status); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CANCELLED intent order status causes indefinite polling
The IntentOrderStatus enum includes CANCELLED as a valid status, but the status mapping in #updateBridgeHistoryFromIntentOrder only includes FAILED and EXPIRED in the isFailed check. When an intent order is cancelled, it falls through to the else branch and gets mapped to StatusTypes.UNKNOWN. Since UNKNOWN is not a terminal status, the polling continues indefinitely and the order appears stuck rather than showing as failed. The CANCELLED status needs to be included in the isFailed array.
Additional Locations (1)
26333b4 to
2b4d03f
Compare
| statusType = StatusTypes.SUBMITTED; | ||
| } else { | ||
| statusType = StatusTypes.UNKNOWN; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CANCELLED intent status maps to UNKNOWN in bridge history
The #updateBridgeHistoryFromIntentOrder method doesn't include IntentOrderStatus.CANCELLED in any of the status checks (isComplete, isFailed, isPending, isSubmitted). When an order is cancelled, it falls through to the else branch and maps to StatusTypes.UNKNOWN. This also means cancelled orders won't stop polling (since isFinal only checks for COMPLETE or FAILED), potentially causing unnecessary API calls. Cancelled orders would more accurately be represented as FAILED since the order definitively won't complete.
| * This is called during initialization | ||
| */ | ||
| readonly #restartPollingForIncompleteHistoryItems = () => { | ||
| readonly #restartPollingForIncompleteHistoryItems = (): void => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intent swap polling not resumed after restart
The #restartPollingForIncompleteHistoryItems method has two issues that prevent intent transactions from resuming polling after browser/extension restart. First, it only checks for PENDING or UNKNOWN status but intent transactions can also have SUBMITTED status (line 406-408). Second, it filters to only restart isBridgeTx (cross-chain) transactions (line 417-421), but #startPollingForTxId also polls same-chain intent swaps via the isIntent check. Same-chain intent swaps that were incomplete at restart will never have their polling resumed.
| default: | ||
| return TransactionStatus.submitted; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CANCELLED intent status incorrectly maps to submitted
The mapIntentOrderStatusToTransactionStatus function doesn't handle IntentOrderStatus.CANCELLED, which is defined in the enum. When an intent order is cancelled, it falls through to the default case and returns TransactionStatus.submitted. This would cause cancelled orders to appear as still pending/submitted in the UI rather than showing as failed, potentially confusing users about the actual state of their transaction.
bfda23e to
fa723b7
Compare
| // Check metadata for additional transaction hashes | ||
| const metadataTxHashes = Array.isArray(intentOrder.metadata.txHashes) | ||
| ? intentOrder.metadata.txHashes | ||
| : []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Single string txHashes from API ignored due to array check
The metadata.txHashes field can be either an array of strings or a single string according to the validator schema (union([array(string()), string()])). However, the code only handles the array case: when txHashes is a single string, Array.isArray() returns false and the code defaults to an empty array, losing the transaction hash data. This could cause incomplete tracking of on-chain settlements for partially-filled intent orders.
| status: (isComplete ? '0x1' : '0x0') as unknown as string, | ||
| }, | ||
| } as Partial<TransactionMeta>) | ||
| : {}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
txReceipt status incorrectly set to failure for pending orders
When updating TransactionController with intent order status, if a txHash exists, the code sets txReceipt.status to '0x0' for any non-complete order (including PENDING and SUBMITTED states). The value '0x0' indicates a reverted/failed on-chain transaction, which will cause other parts of the codebase (like getReceivedERC20Amount) to incorrectly treat in-progress orders as failed transactions. The txReceipt should only be populated with a status for terminal states.
| * This is called during initialization | ||
| */ | ||
| readonly #restartPollingForIncompleteHistoryItems = () => { | ||
| readonly #restartPollingForIncompleteHistoryItems = (): void => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Polling not restarted for SUBMITTED status or same-chain intents
The #restartPollingForIncompleteHistoryItems function only restarts polling for transactions with PENDING or UNKNOWN status, but the new SUBMITTED status type (used for intent transactions) is not included. Additionally, the filter only restarts polling for cross-chain bridge transactions (isBridgeTx), excluding same-chain intent swaps. This means if the controller reinitializes (e.g., browser restart), intent transactions with SUBMITTED status or same-chain intent swaps will never resume polling and will appear stuck to users.
| address: string; | ||
| ignoreNetwork: boolean; | ||
| }) => { | ||
| }): void => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Polling tokens not stopped when wiping with ignoreNetwork
When wipeBridgeStatus is called with ignoreNetwork: true, the txHistory is cleared but active polling tokens in #pollingTokensByTxMetaId are not stopped. This creates a resource leak where background polling continues for transactions that no longer exist in history. The #wipeBridgeStatusByChainId method correctly stops polling before deletion, but the ignoreNetwork path does not.
| * This is called during initialization | ||
| */ | ||
| readonly #restartPollingForIncompleteHistoryItems = () => { | ||
| readonly #restartPollingForIncompleteHistoryItems = (): void => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restart polling misses SUBMITTED status for intent orders
The #restartPollingForIncompleteHistoryItems method only restarts polling for transactions with PENDING or UNKNOWN status, but the new StatusTypes.SUBMITTED status was added for intent orders in this PR. Intent transactions in SUBMITTED status (after order submission but before execution) won't have their polling restarted if the controller is reinitialized (e.g., user closes and reopens the browser). This could cause intent orders to get stuck with no status tracking.
|
|
||
| readonly #addTxToHistory = ( | ||
| startPollingForBridgeTxStatusArgs: StartPollingForBridgeTxStatusArgsSerialized, | ||
| ) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same-chain intent swaps excluded from restart polling filter
The #restartPollingForIncompleteHistoryItems method filters to only restart polling for cross-chain (bridge) transactions via isCrossChain(). However, intent orders can be same-chain swaps, and #startPollingForTxId specifically allows polling for intent transactions via txId.startsWith('intent:'). Same-chain intent swaps won't have their polling restarted on controller reinitialization because they're filtered out before the intent check occurs.
| default: | ||
| return TransactionStatus.submitted; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cancelled intent status incorrectly maps to submitted
The mapIntentOrderStatusToTransactionStatus function doesn't handle IntentOrderStatus.CANCELLED, causing it to fall through to the default case and return TransactionStatus.submitted. A cancelled intent order would incorrectly appear as pending/in-progress to users. Similarly in #updateBridgeHistoryFromIntentOrder, CANCELLED is missing from the isFailed array, causing it to map to StatusTypes.UNKNOWN instead of FAILED. Both locations should treat CANCELLED as a failed state.
Additional Locations (1)
| // Check metadata for additional transaction hashes | ||
| const metadataTxHashes = Array.isArray(intentOrder.metadata.txHashes) | ||
| ? intentOrder.metadata.txHashes | ||
| : []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String txHashes from metadata are discarded
The code extracting transaction hashes only handles the array case: Array.isArray(intentOrder.metadata.txHashes) ? intentOrder.metadata.txHashes : []. According to IntentOrderResponseSchema, txHashes can be string[] | string. When the API returns a single string instead of an array, this code sets metadataTxHashes to an empty array, discarding the transaction hash. This could cause loss of transaction tracking data for intent orders.
| default: | ||
| return TransactionStatus.submitted; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CANCELLED intent status incorrectly maps to submitted
The IntentOrderStatus.CANCELLED enum value is defined but not handled in mapIntentOrderStatusToTransactionStatus. When an intent order is cancelled, the function falls through to the default case and returns TransactionStatus.submitted, which is incorrect. A cancelled order would appear to users as still being submitted/pending rather than terminated. Similarly, in #updateBridgeHistoryFromIntentOrder, CANCELLED is not included in the isFailed array (unlike EXPIRED), causing it to map to StatusTypes.UNKNOWN instead of StatusTypes.FAILED.
Additional Locations (1)
| // Check metadata for additional transaction hashes | ||
| const metadataTxHashes = Array.isArray(intentOrder.metadata.txHashes) | ||
| ? intentOrder.metadata.txHashes | ||
| : []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String txHashes from intent metadata silently dropped
The IntentOrder type declares metadata.txHashes can be either string[] | string, and the schema validates both formats. However, the extraction code only handles the array case using Array.isArray(). When the backend returns a single string instead of an array, the code falls through to [] and the transaction hash is silently discarded. This would cause the srcTxHashes field to be empty when a string hash is returned, leading to missing transaction history data.
Explanation
References
Checklist
Note
Introduces intent-based trading and end-to-end plumbing across controllers, plus status tracking and metrics.
Intent/IntentOrderLike; addStatusTypes.SUBMITTEDIntentOrderSchema/IntentSchemaand response validation; allow digit-string fieldssubmitIntentflow; poll intent order status via newIntentApiImpl(CrossChain API); map intent statuses to bridgePENDING/SUBMITTED/COMPLETE/FAILED; persistoriginalTransactionIdand aggregatesrcTxHashes; start/stop polling forintent:*keys; expanded tests and snapshots; slight Jest coverage threshold tweaksWritten by Cursor Bugbot for commit 20ae6d7. This will update automatically on new commits. Configure here.