Repository navigation
feat(swaps): move cancellation into activity receipts - #827
Conversation
Remove historical swaps and cancellation controls from the swap composer, keeping swap history in recent and full Activity instead. Add the All/Swaps Activity filter and a confirmed receipt-level cancellation flow that persists the cancellation transaction ID and reconciles spend races. Cover filtering, composer access, receipt states, and cancellation persistence with focused tests.
WalkthroughChangesWallet swap flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Wallet as Wallet screen
participant Transaction as Transaction screen
participant AssetSwapsProvider
participant SwapStore
Wallet->>Transaction: open swap transaction
Transaction->>AssetSwapsProvider: read live swap status
Transaction->>AssetSwapsProvider: cancelSwap
AssetSwapsProvider->>SwapStore: persist cancellation status and spentTxid
SwapStore-->>Transaction: updated cancellation state
Transaction-->>Wallet: render cancelled receipt
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying wallet-bitcoin with
|
| Latest commit: |
56e8dae
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://af75d18f.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://wt-swap-activity-controls-20.wallet-bitcoin.pages.dev |
Deploying wallet-mutinynet with
|
| Latest commit: |
56e8dae
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://293cf7bd.arkade-wallet.pages.dev |
| Branch Preview URL: | https://wt-swap-activity-controls-20.arkade-wallet.pages.dev |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/test/providers/assetSwaps.test.tsx (1)
37-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftMissing coverage for the race-reconciliation path.
Only the direct-success
cancelOfferflow is tested. The newresolveCancellingSpendclassification (spent-but-ambiguous cancel-vs-fill, and the monitor's SSE reconciliation) is the core new behavior for this cohort and isn't exercised here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/providers/assetSwaps.test.tsx` around lines 37 - 66, The AssetSwapsProvider cancellation tests only cover direct cancelOffer success and omit resolveCancellingSpend reconciliation. Add tests that exercise the spent-but-ambiguous cancel-versus-fill classification and the monitor’s SSE reconciliation path, asserting the resulting swap status and transaction data while preserving the existing direct-success coverage.src/providers/assetSwaps.tsx (1)
166-181: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo guard against overlapping
cancelOffercalls for the same swap.
cancelSwapdoesn't check whether an invocation foridis already in flight, so a remount + "Retry cancel" while a previous call is still resolving would issue a secondcancelOffercall. The catch'sresolveCancellingSpendfallback covers the case where the deposit is already provably spent, but not other concurrent-call failure modes. Consider an in-flight guard (e.g. a ref-keyedSet) for defense-in-depth, and confirmcancelOffertolerates being invoked twice for the same offer.const inFlightRef = useRef(new Set<string>()) // in cancelSwap: if (inFlightRef.current.has(id)) return; inFlightRef.current.add(id); ...finally { inFlightRef.current.delete(id) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/assetSwaps.tsx` around lines 166 - 181, Prevent overlapping cancellation requests in cancelSwap by adding a ref-backed Set keyed by swap ID, returning immediately when the ID is already in flight, and registering it before invoking cancelOffer. Ensure the ID is removed in a finally block so retries remain possible after success or failure, while preserving the existing cancellation and error-handling flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/providers/assetSwaps.tsx`:
- Around line 190-197: Update the state-revert branch in the asset-swap monitor
around resolveCancellingSpend so it only writes swap.status when the current
swap status is still cancelling. Reuse the same freshness guard used by the
success path and resolveCancellingSpend, preventing stale polling results from
overwriting a newer terminal status.
In `@src/screens/Wallet/Activity.tsx`:
- Around line 15-25: Update the hasSwaps calculation in Activity to use the same
dev-asset visibility criteria as TransactionsList before checking for swap
transactions, so the Swaps toggle only appears when a visible swap exists. Also
ensure the selected filter cannot remain swaps when no visible swaps are
available by resetting it to all, preserving the existing filter-selection
behavior.
---
Nitpick comments:
In `@src/providers/assetSwaps.tsx`:
- Around line 166-181: Prevent overlapping cancellation requests in cancelSwap
by adding a ref-backed Set keyed by swap ID, returning immediately when the ID
is already in flight, and registering it before invoking cancelOffer. Ensure the
ID is removed in a finally block so retries remain possible after success or
failure, while preserving the existing cancellation and error-handling flow.
In `@src/test/providers/assetSwaps.test.tsx`:
- Around line 37-66: The AssetSwapsProvider cancellation tests only cover direct
cancelOffer success and omit resolveCancellingSpend reconciliation. Add tests
that exercise the spent-but-ambiguous cancel-versus-fill classification and the
monitor’s SSE reconciliation path, asserting the resulting swap status and
transaction data while preserving the existing direct-success coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5376c859-b237-4fa3-95b9-e0f4dfbcaa43
📒 Files selected for processing (16)
src/components/TransactionsList.tsxsrc/index.csssrc/lib/swap/restore.tssrc/lib/swap/store.tssrc/providers/assetSwaps.tsxsrc/screens/Wallet/Activity.tsxsrc/screens/Wallet/BitcoinDetail.tsxsrc/screens/Wallet/HomeQuickActions.tsxsrc/screens/Wallet/Swap/Index.tsxsrc/screens/Wallet/Transaction.tsxsrc/test/components/transactions-list.test.tsxsrc/test/providers/assetSwaps.test.tsxsrc/test/screens/wallet/activity.test.tsxsrc/test/screens/wallet/index.test.tsxsrc/test/screens/wallet/swap.test.tsxsrc/test/screens/wallet/transaction.test.tsx
|
Perfect thanks! Remaining task for me |
Reuse the Activity filter control on individual asset pages and scope swap visibility to the selected asset. Cover the bitcoin asset history filtering path with a focused screen test. Closes #830
Use the transaction list visibility rule when exposing swap filters and reset stale selections when visible swaps disappear. Prevent cancellation recovery from overwriting a newer terminal status and cover both regressions.


Summary
This is a focused follow-up to #784 for #796 and #806. Those issues are intentionally referenced without closing keywords because they should close only after this work is merged into the parent branch. This asset-page follow-up closes #830 when the PR merges.
User-visible behavior
Visual proof
Mutinynet preview: https://wt-swap-activity-controls-20.arkade-wallet.pages.dev
Verification
bun run test— 61 files passed, 441 tests passed, 3 skipped.bun run lintbun run format:checkbunx tsc --noEmitgit diff --check56e8daea: hidden swap visibility/filter reset and stale cancellation status protection.Complete file scope
src/components/ActivityFilter.tsx— share the existing accessible, animated, haptic All/Swaps chip control across activity surfaces.src/components/TransactionsList.tsx— add optional transaction-type filtering after the existing visibility filters and expose the existing asset and visibility matchers for consistent swap detection.src/index.css— add token-based Activity filter styling and remove obsolete composer cancellation styles.src/lib/swap/restore.ts— expose cancellation-spend classification for live reconciliation.src/lib/swap/store.ts— update the outdated consumer comment.src/providers/assetSwaps.tsx— persist cancellation transaction IDs, reconcile terminal cancellation states, and prevent stale recovery results from overwriting newer terminal states.src/screens/Wallet/Activity.tsx— use the shared conditional All/Swaps filter while preserving selection feedback, animation, and reduced-motion handling; gate it with the list visibility rule and reset stale selection when visible swaps disappear.src/screens/Wallet/BitcoinDetail.tsx— make composer entry depend on live swap availability and add asset-scoped All/Swaps filtering to Recent activity with the same visibility gating and stale-selection reset.src/screens/Wallet/HomeQuickActions.tsx— apply the same composer availability behavior.src/screens/Wallet/Swap/Index.tsx— remove swap history and inline cancellation from the composer.src/screens/Wallet/Transaction.tsx— add receipt cancellation confirmation, progress, retry, error, success, and live reconciliation states.src/test/components/transactions-list.test.tsx— cover transaction-type filtering.src/test/providers/assetSwaps.test.tsx— cover cancellation persistence, reload behavior, and protection against stale cancellation recovery writes.src/test/screens/wallet/activity.test.tsx— cover the default filter, swap-only results, conditional visibility, and reset behavior when the last visible swap disappears.src/test/screens/wallet/bitcoin-detail.test.tsx— verify All shows mixed bitcoin activity and Swaps keeps only bitcoin-involving swaps.src/test/screens/wallet/index.test.tsx— verify historical swaps do not enter an unavailable composer.src/test/screens/wallet/swap.test.tsx— verify the composer excludes history and cancellation controls.src/test/screens/wallet/transaction.test.tsx— cover confirmation, progress, success, retry, and stale-error reconciliation.No dependencies or unrelated product behavior are changed. Local environment data, test-wallet credentials, screenshots, and review transcripts are not committed to the product branch.
Base
This PR intentionally targets
claude/swap-ui-mutinynet-yktp51, the current head branch of #784, rather thanmaster.