Skip to content

fix: prevent swap autoclaim race between service worker and UI #558

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.

Sign up for GitHub

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

Open
Kukks wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: prevent swap autoclaim race between service worker and UI #558

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5995fc6
fix: prevent swap autoclaim race by querying service worker state
Kukks Apr 23, 2026
4a6ca3e
Apply suggestions from code review
bordalix Apr 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"test:e2e": "VITE_NOSTR_RELAY_URL=http://localhost:10547 playwright test",
"test:e2e:ui": "VITE_NOSTR_RELAY_URL=http://localhost:10547 playwright test --ui",
"test:codegen": "playwright codegen localhost:3002",
"lint": "eslint 'src/**/*.{ts,tsx,js,jsx}'",
"lint": "eslint \"src/**/*.{ts,tsx,js,jsx}\"",
"lint:fix": "eslint 'src/**/*.{ts,tsx,js,jsx}' --fix",
Comment thread
bordalix marked this conversation as resolved.
Outdated
"format": "prettier --write src",
"format:check": "prettier --check src",
Expand Down
19 changes: 16 additions & 3 deletions src/screens/Apps/Boltz/Swap.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,25 @@ export default function AppBoltzSwap() {
const [processing, setProcessing] = useState<boolean>(false)
const [opDone, setOpDone] = useState(false)
const [success, setSuccess] = useState<boolean>(false)
const [swapManagerProcessing, setSwapManagerProcessing] = useState(false)

// Subscribe to real-time updates for this swap
useEffect(() => {
if (!swapManager || !swapInfo) return

let unsub: (() => void) | null = null
let cancelled = false

swapManager.isProcessing(swapInfo.id).then((p) => {
if (!cancelled) setSwapManagerProcessing(p)
})

swapManager
.subscribeToSwapUpdates(swapInfo.id, (updatedSwap) => {
setSwapInfo(updatedSwap)
swapManager.isProcessing(updatedSwap.id).then((p) => {
if (!cancelled) setSwapManagerProcessing(p)
})
Comment on lines +63 to +72
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's check the actual file and its imports
head -50 src/screens/Apps/Boltz/Swap.tsx

Repository: arkade-os/wallet

Length of output: 2284

🏁 Script executed:

# Read the specific lines mentioned in the review (63-72) and surrounding context
sed -n '50,85p' src/screens/Apps/Boltz/Swap.tsx

Repository: arkade-os/wallet

Length of output: 1106

🏁 Script executed:

# Check all imports in the file to verify consoleError is available
grep -n "^import" src/screens/Apps/Boltz/Swap.tsx | head -20

Repository: arkade-os/wallet

Length of output: 1187

🏁 Script executed:

# Search for other similar unhandled promise patterns in this file
rg -n "\.then\(" src/screens/Apps/Boltz/Swap.tsx | head -20

Repository: arkade-os/wallet

Length of output: 218

Handle failed processing-state checks with proper error containment.

Both isProcessing() calls (lines 63–65 and 70–72) lack .catch() handlers, which leaves unhandled promise rejections and stale swapManagerProcessing state if the swap-manager check fails. This can freeze the claim/refund button UI. Extract a helper function to centralize error handling, log the failure, and reset state to a safe default.

🛡️ Proposed fix 
-    swapManager.isProcessing(swapInfo.id).then((p) => {
-      if (!cancelled) setSwapManagerProcessing(p)
-    })
+    const refreshSwapManagerProcessing = (id: string) => {
+      swapManager
+        .isProcessing(id)
+        .then((p) => {
+          if (!cancelled) setSwapManagerProcessing(p)
+        })
+        .catch((error) => {
+          if (!cancelled) {
+            consoleError(error, `Error checking swap processing state ${id}`)
+            setSwapManagerProcessing(false)
+          }
+        })
+    }
+
+    refreshSwapManagerProcessing(swapInfo.id)
 
     swapManager
       .subscribeToSwapUpdates(swapInfo.id, (updatedSwap) => {
         setSwapInfo(updatedSwap)
-        swapManager.isProcessing(updatedSwap.id).then((p) => {
-          if (!cancelled) setSwapManagerProcessing(p)
-        })
+        refreshSwapManagerProcessing(updatedSwap.id)
       })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
swapManager.isProcessing(swapInfo.id).then((p) => {
if (!cancelled) setSwapManagerProcessing(p)
})
swapManager
.subscribeToSwapUpdates(swapInfo.id, (updatedSwap) => {
setSwapInfo(updatedSwap)
swapManager.isProcessing(updatedSwap.id).then((p) => {
if (!cancelled) setSwapManagerProcessing(p)
})
const refreshSwapManagerProcessing = (id: string) => {
swapManager
.isProcessing(id)
.then((p) => {
if (!cancelled) setSwapManagerProcessing(p)
})
.catch((error) => {
if (!cancelled) {
consoleError(error, `Error checking swap processing state ${id}`)
setSwapManagerProcessing(false)
}
})
}
refreshSwapManagerProcessing(swapInfo.id)
swapManager
.subscribeToSwapUpdates(swapInfo.id, (updatedSwap) => {
setSwapInfo(updatedSwap)
refreshSwapManagerProcessing(updatedSwap.id)
})
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/screens/Apps/Boltz/Swap.tsx` around lines 63 - 72, The two calls to
swapManager.isProcessing (inside the initial check and inside the
subscribeToSwapUpdates callback) lack error handling; extract a helper like
safeCheckProcessing(swapId) that calls swapManager.isProcessing(swapId).then(p
=> { if (!cancelled) setSwapManagerProcessing(p) }).catch(err => {
console.error('isProcessing failed', err, swapId); if (!cancelled)
setSwapManagerProcessing(false) }) and replace both direct calls with
safeCheckProcessing(swapInfo.id) so failures are logged and
swapManagerProcessing is reset to a safe default.

})
.then((unsubscribe) => {
if (cancelled) {
Expand Down Expand Up @@ -147,11 +156,15 @@ export default function AppBoltzSwap() {

const isRefundable = isSubmarineSwapRefundable(swapInfo) || isChainSwapRefundable(swapInfo)
const isClaimable = isReverseSwapClaimable(swapInfo) || isChainSwapClaimable(swapInfo)
const buttonLabel = isClaimable ? 'Complete swap' : 'Refund swap'
const buttonLabel = swapManagerProcessing ? 'Claiming...' : isClaimable ? 'Complete swap' : 'Refund swap'
const refunded = swapInfo.status === 'transaction.refunded'

const buttonHandler = async () => {
try {
if (swapManager) {
const alreadyProcessing = await swapManager.isProcessing(swapInfo.id)
if (alreadyProcessing) return
}
Comment on lines +159 to +167
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Scope the guard to claims and sync stale UI on early return.

The current swapManagerProcessing handling also disables/short-circuits refund flows, even though this guard is meant for auto-claim races. Also, when the handler discovers alreadyProcessing, it returns without setting swapManagerProcessing, so a stale UI can remain enabled.

🐛 Proposed fix 
-  const buttonLabel = swapManagerProcessing ? 'Claiming...' : isClaimable ? 'Complete swap' : 'Refund swap'
+  const claimInProgress = swapManagerProcessing && !isRefundable
+  const buttonLabel = claimInProgress ? 'Claiming...' : isClaimable ? 'Complete swap' : 'Refund swap'
   const refunded = swapInfo.status === 'transaction.refunded'
 
   const buttonHandler = async () => {
     try {
-      if (swapManager) {
+      if (isClaimable && swapManager) {
         const alreadyProcessing = await swapManager.isProcessing(swapInfo.id)
-        if (alreadyProcessing) return
+        if (alreadyProcessing) {
+          setSwapManagerProcessing(true)
+          return
+        }
       }
-      {!success && (isRefundable || isClaimable || swapManagerProcessing) ? (
+      {!success && (isRefundable || isClaimable || claimInProgress) ? (
         <ButtonsOnBottom>
-          <Button onClick={buttonHandler} label={buttonLabel} disabled={processing || swapManagerProcessing} />
+          <Button onClick={buttonHandler} label={buttonLabel} disabled={processing || claimInProgress} />
         </ButtonsOnBottom>
       ) : null}

Also applies to: 233-235

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/screens/Apps/Boltz/Swap.tsx` around lines 159 - 167, The guard that calls
swapManager.isProcessing should only run for claim flows (when isClaimable is
true) and when it detects alreadyProcessing it must update swapManagerProcessing
to keep the UI in sync before returning; update the buttonHandler (and the
analogous handler around the isClaimable/refund branch at the other spot) to: 1)
only call swapManager.isProcessing when isClaimable is true, and 2) if
alreadyProcessing is true set swapManagerProcessing (the state/flag used to
disable the UI) appropriately (e.g., set to true) before returning so the button
label/disabled state reflects the ongoing manager processing.

setProcessing(true)
if (isReverseSwapClaimable(swapInfo)) {
await claimVHTLC(swapInfo)
Expand Down Expand Up @@ -217,9 +230,9 @@ export default function AppBoltzSwap() {
)}
</Padded>
</Content>
{!success && (isRefundable || isClaimable) ? (
{!success && (isRefundable || isClaimable || swapManagerProcessing) ? (
<ButtonsOnBottom>
<Button onClick={buttonHandler} label={buttonLabel} disabled={processing} />
<Button onClick={buttonHandler} label={buttonLabel} disabled={processing || swapManagerProcessing} />
</ButtonsOnBottom>
) : null}
</>
Expand Down
Loading