[bordalix]

Fixes values in submarine swap page and direction in reverse swap.

Summary by CodeRabbit

• Bug Fixes

  • Corrected reverse-swap label to "Reverse Swap" and improved handling of missing swap amounts so displayed values default to 0.
  • Fixed displayed amount/fee/total calculations for submarine and reverse swaps.

• Tests

  • Expanded end-to-end coverage: added detailed assertions for swap detail fields and values, and a new on-chain (Arkade to Bitcoin) swap test.

[coderabbitai]

Walkthrough

Defaults null reverse-swap onchain amounts to 0 and renames its Kind to "Reverse Swap". For submarine swaps, uses response.expectedAmount for sent amount and derives received amount from a decoded invoice only when valid. Adds/adjusts end-to-end tests asserting updated swap detail fields and values.

Changes

Cohort / File(s)	Summary
Boltz Swap UI logic src/screens/Apps/Boltz/Swap.tsx	Reverse swap: fallback rcvdSats to 0 when response.onchainAmount is null/undefined and change Kind label to "Reverse Swap". Submarine swap: compute sentSats from response.expectedAmount; rcvdSats derived from decoded invoice only if valid.
End-to-end tests src/test/e2e/swap.test.ts	Removed a timeout; expanded tests to navigate into specific swap details and assert visibility and exact values for multiple fields (Kind, Direction, Status, Amount, Fees, Total) for Lightning (reverse/submarine) and added a new Bitcoin (chain) swap test.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• adds invoice description and address to swap page #248: Modifies Swap.tsx with related swap amount and invoice-derived field logic.
• Swap integration tests #230: Touches the same e2e test file and swap UI expectations in tests.
• restore swaps from boltz endpoint #282: Updates Swap.tsx handling of invoice/amount display similar to these changes.

Suggested reviewers

• Kukks

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'fix swap page' is vague and generic, using non-descriptive language that doesn't convey the specific changes made (submarine swap value fixes and reverse swap direction corrections).	Consider using a more specific title like 'Fix submarine swap values and reverse swap direction display' to clearly communicate the main changes.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/swap_page

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cloudflare-workers-and-pages]

Deploying wallet-bitcoin with    Cloudflare Pages

Latest commit:	2368aec
Status:	✅  Deploy successful!
Preview URL:	https://a7fcd01e.wallet-bitcoin.pages.dev
Branch Preview URL:	https://fix-swap-page.wallet-bitcoin.pages.dev

View logs

[cloudflare-workers-and-pages]

Deploying wallet-mutinynet with    Cloudflare Pages

Latest commit:	2368aec
Status:	✅  Deploy successful!
Preview URL:	https://ed8e4082.arkade-wallet.pages.dev
Branch Preview URL:	https://fix-swap-page.arkade-wallet.pages.dev

View logs

[ghost]

✅ Approved — UI-only display fixes, all correct

Reviewed the full diff, the complete Swap.tsx file, and the upstream boltz-swap type definitions (CreateSubmarineSwapRequest, CreateSubmarineSwapResponse, CreateReverseSwapRequest, CreateReverseSwapResponse).

Changes reviewed

1. Reverse swap kind label (line 118)
'Chain Swap' → 'Reverse Swap' — clearly a copy-paste bug from the chain swap block above. Fix is correct.

2. Nullish coalescing on reverse swap amounts (lines 113–114)
invoiceAmount ?? 0 and onchainAmount ?? 0 — both fields are typed as number (not optional) in CreateReverseSwapRequest / CreateReverseSwapResponse, so these should never be nullish in practice. Defensive coding, harmless.

3. Submarine swap sent/received swap (lines 130–131)
This is the substantive fix. For a submarine swap (Arkade → Lightning):

• response.expectedAmount = sats the user sends to the ARK lockup address (includes Boltz fees) → correctly assigned to sentSats
• decodeInvoice(request.invoice).amountSats = the Lightning invoice amount the recipient gets → correctly assigned to rcvdSats

The old code had these inverted, which made Fees = sentSats - rcvdSats show a negative or incorrect fee. Fix is correct.

Scope assessment

• No protocol-critical code — purely display logic in the swap detail view
• No cross-repo impact — no public API, type, or interface changes
• No security concerns — no key material, signing, or transaction logic touched

LGTM. Ship it.

[ghost]

✅ Approved — UI-only display fixes, all correct

Reviewed the full diff, the complete Swap.tsx file, and the upstream boltz-swap type definitions (CreateSubmarineSwapRequest, CreateSubmarineSwapResponse, CreateReverseSwapRequest, CreateReverseSwapResponse).

Changes reviewed

1. Reverse swap kind label (line 118)
'Chain Swap' → 'Reverse Swap' — clearly a copy-paste bug from the chain swap block above. Fix is correct.

2. Nullish coalescing on reverse swap amounts (lines 113–114)
invoiceAmount ?? 0 and onchainAmount ?? 0 — both fields are typed as number (not optional) in CreateReverseSwapRequest / CreateReverseSwapResponse, so these should never be nullish in practice. Defensive coding, harmless.

3. Submarine swap sent/received swap (lines 130–131)
This is the substantive fix. For a submarine swap (Arkade → Lightning):

• response.expectedAmount = sats the user sends to the ARK lockup address (includes Boltz fees) → correctly assigned to sentSats
• decodeInvoice(request.invoice).amountSats = the Lightning invoice amount the recipient gets → correctly assigned to rcvdSats

The old code had these inverted, which made Fees = sentSats - rcvdSats show a negative or incorrect fee. Fix is correct.

Scope assessment

• No protocol-critical code — purely display logic in the swap detail view
• No cross-repo impact — no public API, type, or interface changes
• No security concerns — no key material, signing, or transaction logic touched

LGTM. Ship it.

[cloudflare-workers-and-pages]

Deploying tmp-boltz-upstream-mainnet-arkade-wallet with    Cloudflare Pages

Latest commit:	2368aec
Status:	✅  Deploy successful!
Preview URL:	https://bc48427d.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev
Branch Preview URL:	https://fix-swap-page.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev

View logs

[ghost]

Correction to my review above: I was wrong on point 2.

CreateReverseSwapResponse.onchainAmount is typed as onchainAmount?: number (optional) in the current boltz-swap master — so onchainAmount ?? 0 is not merely defensive, it's actually correct and necessary. CreateReverseSwapRequest.invoiceAmount remains number (required), but the response-side field can be undefined.

Sorry for the noise — the nullish coalescing on onchainAmount is the right call.

[ghost]

Iterative review — new commit 78b4239d ("lint")

Reviewed the delta since my last approval. The lint commit removes two ?? 0 null-coalescing guards from sentSats assignments:

• Reverse swap (swapInfo.request.invoiceAmount): type is non-optional number → guard was unnecessary
• Submarine swap (swapInfo.response.expectedAmount): type is non-optional number → guard was unnecessary

onchainAmount ?? 0 is correctly retained since that field's type is optional.

All changes remain display-only (swap details page). No protocol-critical code paths affected. LGTM. ✅

[ghost]

Iterative review — new commit 2368aecf ("add tests")

Reviewed the delta since my last review. The new commit adds E2E assertions for swap detail pages (reverse, submarine, chain) and a new chain swap test. Good coverage addition — this directly validates the display fixes from the earlier commits.

One concern

test.setTimeout(60000) removal — src/test/e2e/swap.test.ts:21: The "should receive funds from Lightning" test previously had a 60s timeout. This was removed but the test still does docker exec lnd lncli payinvoice + waitForPaymentReceived + the new detail page assertions. If Playwright's default timeout (30s) isn't enough, this will flake. The refund test at line ~160 still keeps its test.setTimeout(60000). Was this removal intentional?

The new "should send funds to Bitcoin" test (line ~98) also does wallet funding + chain swap + detail verification with no extended timeout — same risk.

Otherwise clean

• Table component renders testId={title}, so getByTestId('Kind') etc. will resolve correctly. ✓
• Hardcoded amounts (1992/8/2000 for reverse, 1000/1/1001 for submarine, 2111/164/2275 for chain) are standard for a controlled regtest Boltz environment. ✓
• No protocol-critical changes in this commit — tests only.

LGTM. Maintaining my approval.

[coderabbitai]

🧹 Nitpick comments (3)

src/test/e2e/swap.test.ts (2)

138-148: New Bitcoin test misses Preimage and BTC Address label assertions.

The Chain Swap detail table in src/screens/Apps/Boltz/Swap.tsx (lines 99-111) renders both Preimage and BTC Address rows, but this new test only verifies a subset of labels. For parity with the other two swap-detail tests and to catch regressions on those rows, please add visibility checks for them too.

🧪 Proposed addition

   await expect(page.getByText('Direction')).toBeVisible()
   await expect(page.getByText('Date')).toBeVisible()
+  await expect(page.getByText('Preimage')).toBeVisible()
+  await expect(page.getByText('BTC Address')).toBeVisible()
   await expect(page.getByText('Status')).toBeVisible()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/e2e/swap.test.ts` around lines 138 - 148, The test missing
assertions for the Chain Swap detail table should be updated to assert
visibility of the "Preimage" and "BTC Address" rows; in the existing test block
in src/test/e2e/swap.test.ts (the sequence that clicks 'Arkade to Bitcoin' and
checks labels), add two checks using page.getByText('Preimage') and
page.getByText('BTC Address') with await expect(...).toBeVisible() so the test
matches the Swap component (src/screens/Apps/Boltz/Swap.tsx) rendering of those
rows.

---

58-58: Inconsistent exact flag when matching the "Invoice" label.

Line 58 uses getByText('Invoice', { exact: true }) while the equivalent assertion on line 107 omits exact: true. Today the long lnbcrt… invoice string doesn't contain the word "Invoice" so both happen to resolve, but aligning the locators avoids a flaky strict-mode violation if the value rendering ever changes (e.g. truncation, tooltip text).

🧪 Proposed tweak

-  await expect(page.getByText('Invoice')).toBeVisible()
+  await expect(page.getByText('Invoice', { exact: true })).toBeVisible()

Also applies to: 107-107

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/e2e/swap.test.ts` at line 58, The two Playwright locators for the
"Invoice" label are inconsistent—one uses page.getByText('Invoice', { exact:
true }) while the other omits the exact flag—so update the other occurrence in
swap.test.ts to match and make both uses consistent (preferably use
page.getByText('Invoice', { exact: true }) for both) to avoid flaky strict-mode
matches; locate both calls to page.getByText('Invoice', ...) and apply the same
{ exact: true } option.

src/screens/Apps/Boltz/Swap.tsx (1)

130-131: Decode invoice once and guard against amountless invoices.

Two small concerns on the submarine branch:

1. decodeInvoice is invoked twice for the same invoice — once inside isValidInvoice (see src/lib/bolt11.ts:31-38) and again on line 131. Decoding once avoids redundant work.
2. If the invoice is amountless, decodeInvoice(...).amountSats can be undefined, which makes rcvdSats undefined and then sentSats - rcvdSats (line 143) evaluates to NaN, rendering a bad "Fees"/"Amount" cell. Submarine swaps are expected to always carry an amount, but a defensive fallback keeps the UI safe.

♻️ Proposed refactor

-    const sentSats = swapInfo.response.expectedAmount
-    const rcvdSats = isValidInvoice(swapInfo.request.invoice) ? decodeInvoice(swapInfo.request.invoice).amountSats : 0
+    const sentSats = swapInfo.response.expectedAmount ?? 0
+    const decoded = isValidInvoice(swapInfo.request.invoice) ? decodeInvoice(swapInfo.request.invoice) : null
+    const rcvdSats = decoded?.amountSats ?? 0

🤖 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 130 - 131, Decode the invoice
once and defensively handle amountless invoices: replace the double-call pattern
by first checking isValidInvoice(swapInfo.request.invoice) and, if valid, store
the result of decodeInvoice(swapInfo.request.invoice) in a local (e.g.,
decodedInvoice), then set rcvdSats = decodedInvoice?.amountSats ?? 0; keep
sentSats = swapInfo.response.expectedAmount. Reference: sentSats, rcvdSats,
swapInfo.request.invoice, isValidInvoice, decodeInvoice.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/screens/Apps/Boltz/Swap.tsx`:
- Around line 130-131: Decode the invoice once and defensively handle amountless
invoices: replace the double-call pattern by first checking
isValidInvoice(swapInfo.request.invoice) and, if valid, store the result of
decodeInvoice(swapInfo.request.invoice) in a local (e.g., decodedInvoice), then
set rcvdSats = decodedInvoice?.amountSats ?? 0; keep sentSats =
swapInfo.response.expectedAmount. Reference: sentSats, rcvdSats,
swapInfo.request.invoice, isValidInvoice, decodeInvoice.

In `@src/test/e2e/swap.test.ts`:
- Around line 138-148: The test missing assertions for the Chain Swap detail
table should be updated to assert visibility of the "Preimage" and "BTC Address"
rows; in the existing test block in src/test/e2e/swap.test.ts (the sequence that
clicks 'Arkade to Bitcoin' and checks labels), add two checks using
page.getByText('Preimage') and page.getByText('BTC Address') with await
expect(...).toBeVisible() so the test matches the Swap component
(src/screens/Apps/Boltz/Swap.tsx) rendering of those rows.
- Line 58: The two Playwright locators for the "Invoice" label are
inconsistent—one uses page.getByText('Invoice', { exact: true }) while the other
omits the exact flag—so update the other occurrence in swap.test.ts to match and
make both uses consistent (preferably use page.getByText('Invoice', { exact:
true }) for both) to avoid flaky strict-mode matches; locate both calls to
page.getByText('Invoice', ...) and apply the same { exact: true } option.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a485200b-4330-4fd2-b3d0-a87aad2d8044

📥 Commits

Reviewing files that changed from the base of the PR and between 4de95fb and 2368aec.

📒 Files selected for processing (2)

• src/screens/Apps/Boltz/Swap.tsx
• src/test/e2e/swap.test.ts