Use SwapManager #265
Use SwapManager #265
Conversation
Deploying wallet-mutinynet with
|
| Latest commit: |
3c6f67c
|
| Status: | ⚡️ Build in progress... |
|
Caution Review failedThe pull request is closed. WalkthroughReplaces the legacy LightningSwapProvider with ArkadeLightning and SwapManager; refactors LightningContext to expose arkadeLightning, swapManager, and helper methods; updates init/fees/history flows and all call sites to use the new APIs; removes src/lib/lightning.ts. Changes
Sequence Diagram(s)mermaid Note over UI,Ctx: User triggers swap actions / views Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
src/providers/lightning.tsx
Outdated
| }) | ||
| setLogger({ | ||
| log: (...args: unknown[]) => consoleLog(...args ), | ||
| error: (...args: unknown[]) => consoleError(...args ), |
There was a problem hiding this comment.
Incompatible signature for consoleError()
src/providers/lightning.tsx
Outdated
| setLogger({ | ||
| log: (...args: unknown[]) => consoleLog(...args ), | ||
| error: (...args: unknown[]) => consoleError(...args ), | ||
| warn: (...args: unknown[]) => consoleWarn(...args), |
There was a problem hiding this comment.
consoleWarn() doesn't exist
Deploying wallet-bitcoin with
|
| Latest commit: |
4d3743f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://da79ff27.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://swap-manager.wallet-bitcoin.pages.dev |
|
tACK |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/screens/Wallet/Receive/QrCode.tsx (1)
53-81: Verify the dependency array is intentionally minimal.The effect uses
wallet,svcWallet,validLnSwap,createReverseSwap,recvInfo,setRecvInfo, andnavigatebut only lists[satoshis, arkadeLightning]in dependencies. If any of these context values are not referentially stable, stale closures could occur.If the intent is to only trigger the swap creation when amount changes or lightning connects, consider using refs for non-reactive values or adding an exhaustive deps comment.
src/providers/lightning.tsx (1)
177-179: Consider a type guard for cleaner error handling.The
(e as any)?.isRefundablepattern works but could be more type-safe. Consider defining a type guard or error interface if the library exports one.- } catch (e: unknown) { - const refundable = typeof (e as any)?.isRefundable === 'boolean' ? (e as any).isRefundable : false - if (!refundable) throw new Error('Swap failed: VHTLC not refundable') + } catch (e: unknown) { + const isRefundableError = (err: unknown): err is { isRefundable: boolean } => + typeof err === 'object' && err !== null && 'isRefundable' in err + const refundable = isRefundableError(e) && e.isRefundable + if (!refundable) throw new Error('Swap failed: VHTLC not refundable')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
package.json(2 hunks)src/components/SwapsList.tsx(1 hunks)src/lib/lightning.ts(0 hunks)src/providers/lightning.tsx(2 hunks)src/providers/limits.tsx(4 hunks)src/screens/Apps/Boltz/Index.tsx(2 hunks)src/screens/Apps/Boltz/Settings.tsx(2 hunks)src/screens/Apps/Boltz/Swap.tsx(3 hunks)src/screens/Settings/Support.tsx(2 hunks)src/screens/Wallet/Receive/QrCode.tsx(3 hunks)src/screens/Wallet/Send/Details.tsx(2 hunks)src/screens/Wallet/Send/Form.tsx(3 hunks)src/test/screens/mocks.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/lib/lightning.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-07T14:41:35.991Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 106
File: src/lib/bolt11.ts:153-155
Timestamp: 2025-07-07T14:41:35.991Z
Learning: In src/lib/bolt11.ts, the code is copied from the original bitcoinjs/bolt11 library (https://github.com/bitcoinjs/bolt11/blob/master/payreq.js) and should be kept unchanged to maintain compatibility with the upstream source, even if there are potential improvements or syntax issues.
Applied to files:
src/screens/Apps/Boltz/Index.tsxsrc/screens/Wallet/Send/Details.tsx
📚 Learning: 2025-06-30T18:33:29.839Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 114
File: src/lib/asp.ts:0-0
Timestamp: 2025-06-30T18:33:29.839Z
Learning: In src/lib/asp.ts, only the collaborativeExit function should accept both IWallet and ServiceWorkerWallet types. Other wallet functions (getBalance, getTxHistory, getReceivingAddresses, redeemNotes, sendOnChain, settleVtxos) should maintain their original IWallet-only signatures and not be updated for consistency.
Applied to files:
src/screens/Wallet/Receive/QrCode.tsxsrc/screens/Wallet/Send/Details.tsxsrc/screens/Wallet/Send/Form.tsx
🧬 Code graph analysis (10)
src/providers/limits.tsx (1)
src/providers/lightning.tsx (1)
LightningContext(45-62)
src/screens/Settings/Support.tsx (1)
src/providers/lightning.tsx (1)
LightningContext(45-62)
src/screens/Apps/Boltz/Index.tsx (1)
src/providers/lightning.tsx (1)
LightningContext(45-62)
src/screens/Wallet/Receive/QrCode.tsx (1)
src/providers/lightning.tsx (1)
LightningContext(45-62)
src/screens/Apps/Boltz/Settings.tsx (1)
src/providers/lightning.tsx (1)
LightningContext(45-62)
src/components/SwapsList.tsx (1)
src/lib/logs.ts (1)
consoleError(40-44)
src/screens/Wallet/Send/Details.tsx (1)
src/providers/lightning.tsx (1)
LightningContext(45-62)
src/screens/Wallet/Send/Form.tsx (1)
src/providers/lightning.tsx (1)
LightningContext(45-62)
src/screens/Apps/Boltz/Swap.tsx (2)
src/providers/flow.tsx (1)
FlowContext(89-104)src/providers/lightning.tsx (1)
LightningContext(45-62)
src/providers/lightning.tsx (3)
src/providers/asp.tsx (1)
AspContext(15-20)src/providers/wallet.tsx (1)
WalletContext(42-56)src/lib/asp.ts (1)
sendOffChain(231-233)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: wallet-mutinynet
🔇 Additional comments (23)
package.json (2)
30-30: LGTM! Improved error handling with && chaining.Switching from semicolons to
&&ensures that each command succeeds before proceeding, preventing builds with stale workers or missing git info.Also applies to: 32-32
81-81: cross-env@10.1.0 is valid and has no known security vulnerabilities.Version 10.1.0 is the current latest version and is properly maintained with MIT license. No security advisories are reported for this version.
src/screens/Settings/Support.tsx (1)
24-24: LGTM!The migration from
swapProvider.getApiUrl()to the directgetApiUrl()method is clean, and the null fallback on line 76 properly handles the case when Lightning is not initialized.Also applies to: 76-76
src/screens/Wallet/Send/Details.tsx (1)
27-27: LGTM!The migration simplifies the call chain by using
payInvoicedirectly from context. The error handling via.catch(handleError)will properly catch and display the "Lightning not initialized" error if the context isn't ready, though the flow guards (lines 104-107) should prevent this scenario.Also applies to: 108-108
src/screens/Apps/Boltz/Settings.tsx (1)
11-12: LGTM!Clean migration to the new context API with proper null handling for
getApiUrl(). The component rename toAppBoltzSettingsappropriately distinguishes it from the mainAppBoltzindex component.Also applies to: 30-30
src/components/SwapsList.tsx (2)
122-134: Real-time history loading implementation looks good.The effect correctly guards against missing
arkadeLightningand uses async/await pattern with proper error handling. The dependency onarkadeLightningensures the history loads when Lightning becomes available.
136-154: Clean subscription pattern for real-time updates.The subscription correctly handles both updating existing swaps in-place and prepending new swaps. The cleanup function properly unsubscribes when the component unmounts or
swapManagerchanges.src/providers/limits.tsx (3)
44-44: LGTM!Clean destructuring of the new
arkadeLightningandconnectedproperties from the migratedLightningContextAPI.
53-65: LGTM!Guard and dependency array correctly updated to use
arkadeLightning. The effect logic for updating UTXO/VTXO limits remains unchanged.
67-90: LGTM!The swap limits fetch logic correctly migrates from
swapProvidertoarkadeLightning.getLimits(). The dependency array[arkadeLightning, connected]appropriately triggers re-fetch when connection state changes.src/screens/Wallet/Receive/QrCode.tsx (1)
27-27: LGTM!Correctly destructures the new context API, separating the
arkadeLightninginstance from thecreateReverseSwaphelper function.src/screens/Wallet/Send/Form.tsx (3)
51-51: LGTM!Clean destructuring of the new context API surface, replacing direct
swapProviderusage with dedicated helper functions.
279-285: LGTM!The submarine swap creation correctly uses the context-provided
createSubmarineSwapfunction with proper null-check and error handling.
458-464: LGTM!The Boltz nudge correctly uses
getApiUrl()to check if Lightning swaps are configured before showing the enable prompt.src/test/screens/mocks.ts (1)
53-70: LGTM!The mock correctly mirrors the new
LightningContextinterface fromsrc/providers/lightning.tsx. All method stubs have appropriate default return values matching the real context defaults.src/screens/Apps/Boltz/Swap.tsx (3)
28-29: LGTM!Correctly destructures the new context API:
setSwapInfofrom FlowContext for updating swap state, andclaimVHTLC,refundVHTLC,swapManagerfrom LightningContext for swap operations and real-time updates.
36-45: Clean subscription pattern for real-time swap updates.The implementation correctly:
- Guards against missing
swapManagerorswapInfo- Subscribes using the swap ID as the key
- Returns the unsubscribe function for cleanup
- Uses
swapInfo?.idin dependencies to resubscribe only when viewing a different swapOne minor observation:
setSwapInfois used in the callback but not in the dependency array. This is acceptable since context setters are typically stable, but consider adding an eslint-disable comment if your linter flags it.
81-98: LGTM!The button handler correctly delegates to context-provided
claimVHTLCandrefundVHTLCfunctions. The comment on line 92 accurately notes that SwapManager now handles status synchronization, eliminating the need for manual refresh calls.src/providers/lightning.tsx (5)
4-17: LGTM!The imports and BASE_URLS configuration are well-structured. The
testnet: nullplaceholder correctly disables Boltz for that network, and the guard on line 80 handles this appropriately.Also applies to: 19-25
27-62: LGTM!The context interface and defaults are well-designed. Exposing helper methods that delegate to
ArkadeLightningprovides a clean API, and the defaults appropriately handle the uninitialized state.
111-118: LGTM!Clean separation of the fees fetching logic into its own effect with appropriate error handling.
190-225: LGTM!The remaining helper methods correctly guard against null
arkadeLightning, and the provider value properly exposes the complete API surface.
97-101: InconsistentconsoleErrorusage patterns.Line 99 calls
consoleError(args[0], args.slice(1).join(' '))with two arguments, while line 106 passesconsoleErrordirectly to.catch(), which provides only the error object as a single parameter. Verify thatconsoleErroraccepts both one and two-argument calls, or refactor one of these usages for consistency.Also applies to: 106-106
| const unsubscribe = swapManager.onSwapUpdate((swap) => { | ||
| setSwapHistory((prev) => { | ||
| const existingIndex = prev.findIndex((s) => s.id === swap.id) | ||
| if (existingIndex >= 0) { | ||
| const updated = [...prev] | ||
| updated[existingIndex] = swap | ||
| return updated | ||
| } | ||
| // New swap, add to beginning | ||
| return [swap, ...prev] | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for PendingReverseSwap and PendingSubmarineSwap type definitions
fd -t f "\.ts$|\.tsx$" | head -20
rg -n "interface PendingReverseSwap|type PendingReverseSwap" -A 15
rg -n "interface PendingSubmarineSwap|type PendingSubmarineSwap" -A 15
rg -n "\.id\s*:|\.response\.id\s*:" -B 2 -A 2Repository: arkade-os/wallet
Length of output: 599
🏁 Script executed:
#!/bin/bash
# Search for swap type definitions
rg -n "type Pending(Reverse|Submarine)Swap|interface Pending(Reverse|Submarine)Swap" -A 20
# Search for .response.id usage
rg -n "\.response\.id" -B 3 -A 1
# Search for swap.id usage
rg -n "swap\.id" -B 2 -A 1
# Look for the SwapsList.tsx file and check key usage
fd "SwapsList\.tsx" -x cat -nRepository: arkade-os/wallet
Length of output: 8774
Use consistent identifier for swap updates and React keys: swap.id should be used for both.
Line 142 matches swaps by s.id, but line 163 uses swap.response.id as the React key. If these IDs differ, React won't properly reconcile updated swaps—they'll appear as new elements instead of updates, causing UI glitches or stale renders. Use swap.id consistently for both the state update logic and the React key.
🤖 Prompt for AI Agents
In src/components/SwapsList.tsx around lines 140 to 150, the state update
matches swaps by swap.id but elsewhere the JSX uses swap.response.id as the
React key; make the identifiers consistent by changing the key to use swap.id
instead of swap.response.id so React can properly reconcile updated swaps;
update any JSX key or other places referencing swap.response.id to use swap.id
and ensure swap objects always include the canonical id field used here.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Andrew Camilleri <evilkukka@gmail.com>
Requires arkade-os/boltz-swap#41 first
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.