feat: gasless hw batch tracking#30987
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
github-actions
Bot
added
risk:high
mm-token-exchange-service
Bot
added
the
INVALID-PR-TEMPLATE
PR template — items to address before "Ready for review"This check is blocking. Address every item below, then push to re-run.
See docs/readme/ready-for-review.md for the full Definition of Ready for Review. |
github-actions
Bot
added
risk:medium
github-actions
Bot
added
risk:high
| expect(mockAcceptRequest).not.toHaveBeenCalled(); | ||
|
|
||
| act(() => { | ||
| jest.advanceTimersByTime(1_000); |
There was a problem hiding this comment.
We should re-use the constants here, otherwise if we change the constant we would have to change this test too!
…BatchSignTracker - Introduced a new constant for retry delay to improve readability. - Updated the logic for handling transaction approvals and rejections, ensuring late rejections are ignored for already approved transactions. - Added new tests to cover scenarios for retry approvals and stale batch handling, enhancing the robustness of the transaction tracking mechanism. - Refactored existing functions to improve clarity and maintainability, including renaming functions for better context.
github-actions
Bot
added
risk:medium
… useHwBatchSignTracker - Implemented new test cases to ensure that non-terminal bridge transactions from different batches are not aborted when cancelCurrentBatch is called. - Added checks to confirm that untracked non-terminal transactions from the same batch are aborted correctly. - Enhanced the getCancellableBatchTxIds function to consider related batch IDs, improving transaction tracking accuracy.
github-actions
Bot
added
risk:high
| if (request.type === 'transaction_batch') { | ||
| if (relatedBatchIds.has(requestId) || relatedApprovalIds.has(requestId)) { | ||
| return true; | ||
| if (request.type === 'transaction') { |
There was a problem hiding this comment.
Nit: But I think we have a constant for that one now?
| (tx: TransactionMeta) => tx.id === requestId, | ||
| ); | ||
| return Boolean(txMeta && matchesTx(txMeta, targetFrom)); | ||
| if (request.type === 'transaction_batch') { |
There was a problem hiding this comment.
Nit: We should also add a constant for this one!
| trackerState.acceptedApprovalIds = new Set(); | ||
| trackerState.approvalQueue = []; | ||
| trackerState.signedBatchIds = new Set(); | ||
| trackerState.trackedTxIds = new Set(); |
There was a problem hiding this comment.
What "tracked" means here? I saw you were checking for "approved" tx, or is this tx for the current batch?
We could change the name to something else a bit more descriptive otherwise?
| allTxIds, | ||
| trackerState.trackedTxIds, | ||
| ); | ||
| const allTxIds = getCancellableBatchTxIds( |
There was a problem hiding this comment.
Not all tx are really cancellable though? Should we also change the name for this one?
There was a problem hiding this comment.
Yea, these are only the ones that can still be cancelled locally
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit af3f3ea. Configure here.
github-actions
Bot
added
risk:medium
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
Tag selection rationale:
Per tag descriptions: SmokeSwap requires SmokeConfirmations (already included). The changes are backward-compatible but touch critical hardware wallet signing paths. Performance Test Selection: |
2 participants
Description
This PR uuseHwBatchSignTracker which orchestrates hardware-wallet signing for multi-transaction bridge/swap batches by detecting pending approvals from the ApprovalController, queuing them, and driving sequential device confirmation. It anchors a batchId from the first approved transaction, tracks signing progress through transactionStatusUpdated events, and exposes a cancelCurrentBatch function that aborts all in-flight txs, rejects pending approvals, wipes failed tx nonces to prevent gaps, and resets internal state for retry. A generation counter and stale-batch-ID set ensure that late events from cancelled or retried batches are ignored, while special handling covers smart-transaction (STX) submission failures that arrive after signing succeeds.
Changelog
CHANGELOG entry: null
Related issues
Refs: https://consensyssoftware.atlassian.net/browse/MUL-1718
Manual testing steps
N/A — Cannot be tested. its not wired up.
Screenshots/Recordings
Cannot be tested. its not wired up.
Before
After
Pre-merge author checklist
Performance checks (if applicable)
trace()for usage andaddTokenfor an exampleFor performance guidelines and tooling, see the Performance Guide.
Pre-merge reviewer checklist
Note
Medium Risk
New transaction/approval orchestration with cancel, wipe, and STX edge cases is complex and security-adjacent, but the hook is not wired to production flows yet and behavior is heavily unit-tested.
Overview
Adds
useHwBatchSignTracker, a new hook that wires hardware-wallet signing to multi-step bridge/swap transaction batches (not yet integrated into the swaps UI per the PR).When enabled for a
fromAddress, it listens to TransactionController and ApprovalController events, maps bridge/swap batch txs toupdateHardwareWalletsSwapsevents (Signing / Signed / Rejected / TransactionFailed), tracksconfirmationTxId, and exposescancelCurrentBatchto abort in-flight signing, reject scoped pending approvals, drop only pre-broadcast txs, wait for terminal statuses, andwipeTransactionson related chains so failed nonces do not block retries. Stale-batch andretryGenerationRefguards ignore late events from cancelled or retried flows; the approval queue runs throughexecuteHardwareWalletOperationwith special handling for device-not-ready retries, Keystone cancel, and STX `STX_NO_HASH_ERROR** after sign.executeHardwareWalletOperationgains optionalshowConfirmation(skip awaiting-confirmation UI while still running readiness + execute) and exportsHardwareWalletOperationType. Coverage is a large newuseHwBatchSignTracker.test.tssuite plus smallexecuteHardwareWalletOperationtests forshowConfirmation: false.Reviewed by Cursor Bugbot for commit ebee541. Bugbot is set up for automated code reviews on this repo. Configure here.