refactor: extract more bridge-status-controller calls to utils#8215
refactor: extract more bridge-status-controller calls to utils#8215
Conversation
micaelae
force-pushed
the
swaps4229-extract-controller-calls
branch
7 times, most recently
from
8eca356 to
682da9c
Compare
micaelae
force-pushed
the
swaps4229-extract-controller-calls
branch
3 times, most recently
from
4f33f5d to
8cade9a
Compare
squash accounts squash accounts accounts
non evm submit
fix: gas tests refactor: extract more transaction controller utils transaction refactor: move txDataByType to addTransactionBatch util
micaelae
force-pushed
the
swaps4229-extract-controller-calls
branch
3 times, most recently
from
71783df to
72a6c00
Compare
micaelae
changed the title
| jest.spyOn(Date, 'now').mockReturnValueOnce(1234567890); | ||
| jest.spyOn(Date, 'now').mockReturnValueOnce(1234567891); |
There was a problem hiding this comment.
Added Date.now mocks for more accurate actionId snapshot tests, and to make sure actionId usage is preserved after submitTx and submiIntent are unified
| StatusResponse, | ||
| } from '../types'; | ||
|
|
||
| describe('History Utils', () => { |
There was a problem hiding this comment.
Moved these tests from transaction.test.ts
|
|
||
| describe('getIntentFromQuote', () => { | ||
| it('returns intent when present in quote response', () => { | ||
| const mockIntent = { protocol: 'cowswap', order: { some: 'data' } }; |
There was a problem hiding this comment.
Moved from transaction.test.ts
| clientId: BridgeClientId, | ||
| ): Promise<IntentStatusResponse> { | ||
| const endpoint = `${this.#baseUrl}/submitOrder`; | ||
| try { |
There was a problem hiding this comment.
Replaced this with a pure function postSubmitOrder to decouple it from the IntentApi classes. This is in preparation for submitTx+submitIntent unification
| bridgeTxMeta: txMeta, // Only the id field is used by the BridgeStatusController | ||
| statusRequest: { | ||
| ...getStatusRequestParams(quoteResponse), | ||
| srcTxHash: txMeta.hash, |
There was a problem hiding this comment.
The statusRequest's data is also present in quoteResponse and bridgeTxMeta so this is ok to remove
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| } as TransactionMeta; | ||
|
|
||
| const startTime = Date.now(); | ||
| }; |
There was a problem hiding this comment.
Missing originalTransactionId in intent #addTxToHistory call
High Severity
In the intent submission flow, originalTransactionId is set on bridgeTxMetaForHistory as an object property, but bridgeTxMeta is now typed as Pick<TransactionMeta, 'id' | 'hash' | 'batchId'> which doesn't include it. The StartPollingForBridgeTxStatusArgs type has a separate originalTransactionId field, but it's never passed in the #addTxToHistory call. In getInitialHistoryItem, the destructured originalTransactionId from args will be undefined, falling back to bridgeTxMeta?.id (the orderUid) instead of syntheticMeta.id (the TC transaction ID). This breaks syncTransactionFromIntentStatus, which uses originalTransactionId to find and update the correct transaction in the TransactionController.
Additional Locations (1)
| state: { | ||
| txHistory: options?.mockTxHistory ?? {}, | ||
| }, | ||
| addTransactionBatchFn: jest.fn(), |
There was a problem hiding this comment.
Duplicate addTransactionBatchFn property in constructor
Low Severity
The BridgeStatusController constructor object literal has addTransactionBatchFn: jest.fn() specified twice — once at the newly added line and again a few lines later. The second value silently overrides the first. This is likely an accidental addition during the refactor.
| { | ||
| status: bridgeStatus?.transactionStatus, | ||
| hash: txHash, | ||
| ...txReceiptUpdate, |
There was a problem hiding this comment.
Unconditional hash assignment overwrites existing transaction hash
Medium Severity
The old code conditionally set hash only when txHash was truthy via ...(txHash ? { hash: txHash } : {}), preserving any existing hash on the transaction. The new code unconditionally sets hash: txHash, which when txHash is undefined will overwrite an existing valid hash on the TransactionMeta with undefined through the updateTransaction spread ({ ...txMeta, ...txMetaUpdates }).
1 participant
Explanation
Changes
postSubmitOrdersubmitTx + submitIntentrefactorReferences
Fixes https://consensyssoftware.atlassian.net/browse/SWAPS-4229
Checklist
Note
Medium Risk
Refactors transaction submission, gas fee estimation, and intent-order handling into shared utils, which can subtly change bridging/swapping flows and metrics/event tracking behavior. Test updates reduce risk, but the changes touch core transaction lifecycle and status polling paths.
Overview
Refactors
BridgeStatusControllerby moving more TransactionController/metrics/polling logic into dedicated utils. Core submission paths now use new helpers likesubmitEvmTransaction,addTransactionBatch,addSyntheticTransaction, and shared getters/updaters for transaction meta.Intent order submission is decoupled from
IntentApiImplinto a purepostSubmitOrderfunction, and intent-status translation/polling now treats missingtxHashasundefined(not an empty string), updating both controller behavior and snapshots.Gas fee estimation utilities are reorganized so
getTxGasEstimatespulls estimates via messenger calls andgetGasFeeEstimates, with corresponding test refactors.History handling is tightened and rekeying is extracted/expanded, including support for choosing history keys from
actionId,txMeta.id, or intent synthetic IDs, plus new unit tests and updated controller/tests to rely onbridgeTxMeta.hashrather than a separatestatusRequestpayload.Written by Cursor Bugbot for commit ad7a598. This will update automatically on new commits. Configure here.