[sahilc0]

Summary

• Add a bitcoin-only wallet detail page opened from the bitcoin asset row on the redesigned home screen.
• Bring over the draft PR's Liveline-based price chart treatment with current bitcoin price, percentage movement badge, range tabs, holdings balance/value, and bitcoin-filtered recent activity.
• Share the existing Vaul-backed swap coming-soon drawer between the home quick action and the bitcoin detail Swap action.

Affected files and components

• src/screens/Wallet/BitcoinDetail.tsx adds the bitcoin detail screen and chart/price data handling.
• src/screens/Wallet/AssetsSection.tsx routes the bitcoin row into the detail page.
• src/components/SwapComingSoonSheet.tsx centralizes the PR 627 swap coming-soon drawer.
• src/providers/navigation.tsx registers the bitcoin detail route under the wallet tab.
• src/index.css adds the detail page styles ported from the draft approach.
• package.json, bun.lock, and pnpm-lock.yaml add liveline.

Testing

• bunx vitest run src/test/screens/wallet/index.test.tsx
• bun run lint
• bun run build
• Playwright smoke check against wt-bitcoin-detail-page-20260521.arkade.localhost: clicked the bitcoin row, confirmed the detail page rendered with a chart canvas and Receive/Send/Swap/Scan actions, and verified Swap opens the coming-soon drawer.

[sahilc0]

@bordalix this PR brings over the frontend for the bitcoin detail price/chart experience from the draft approach. We may need the app/backend to serve the actual bitcoin price and historical chart data instead of relying directly on external market APIs from the client. Could you help advise on the right way to serve that data?

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)

• next-version

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e170ca60-8303-4bf8-bbc8-828718b49bd2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch wt-bitcoin-detail-page-20260521

---

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:	1820528
Status:	✅  Deploy successful!
Preview URL:	https://460ec5b6.wallet-bitcoin.pages.dev
Branch Preview URL:	https://wt-bitcoin-detail-page-20260.wallet-bitcoin.pages.dev

View logs

[cloudflare-workers-and-pages]

Deploying wallet-mutinynet with    Cloudflare Pages

Latest commit:	1820528
Status:	✅  Deploy successful!
Preview URL:	https://25d95223.arkade-wallet.pages.dev
Branch Preview URL:	https://wt-bitcoin-detail-page-20260.arkade-wallet.pages.dev

View logs

[cloudflare-workers-and-pages]

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

Latest commit:	1820528
Status:	✅  Deploy successful!
Preview URL:	https://fa26cbbb.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev
Branch Preview URL:	https://wt-bitcoin-detail-page-20260.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev

View logs

[ghost]

Code Review — feat(wallet): add bitcoin detail page

Overall this is a well-structured UI feature: clean component decomposition, good accessibility (aria roles on tabs, reduced-motion support), and solid refactoring of SwapComingSoonSheet into a shared component. But there's one crash-level bug that blocks merge.

---

🔴 BUG — convertFiat does not exist (runtime crash)

src/screens/Wallet/BitcoinDetail.tsx:4 (line 594 in diff)

const { convertFiat, fiatDecimals, toFiat } = useContext(FiatContext)

FiatContext exports { toFiat, fromFiat, fiatDecimals, updateFiatPrices }. There is no convertFiat property. Destructuring yields undefined.

This undefined is then passed through useBitcoinMarketChartData → fetchBitcoinMarketChart → fetchCoinbaseBitcoinAllChart → fetchCoinbaseBitcoinCandles, where it's called at BitcoinDetail.tsx:476 (line 1026 in diff):

[time * 1000, fiat === Fiats.USD ? close : convertFiat(close, Fiats.USD)]

Impact: For any user whose fiat currency is not USD, selecting the "All" chart range will throw TypeError: convertFiat is not a function, crashing the component. USD users are accidentally safe because the ternary short-circuits.

Fix: Either add a convertFiat(amount, fromCurrency) method to FiatContext/FiatProvider, or build the conversion inline using the existing toFiat/fromFiat helpers. Something like:

const convertFromUSD = (usdAmount: number) => {
  const sats = fromFiat(usdAmount) // if fromFiat were currency-aware...
  return toFiat(sats)
}

But note fromFiat uses the user's configured fiat, not a specified source currency — so you likely need a new helper that takes a source currency parameter, or fetch Coinbase candles in USD and convert via the existing price ratio.

---

🟡 Coinbase "All" chart — unbounded parallel requests

BitcoinDetail.tsx:442-448 (lines 992-997 in diff)

for (let start = ALL_CHART_START_TIME; start < now; start += chunkSecs) {
  requests.push(fetchCoinbaseBitcoinCandles(start, end, fiat, convertFiat, signal))
}
const chunks = await Promise.all(requests)

From 2015 to 2026 with 300-day chunks = ~14 parallel requests to api.exchange.coinbase.com. Coinbase's public API rate limit is 10 req/s per IP. This will likely trigger 429s on slower connections or under any existing load.

Suggestion: Use Promise.allSettled with a concurrency limiter (e.g., batch 3-4 at a time), or fetch sequentially. Also handle 429 responses with a retry/backoff rather than throwing.

---

🟡 No caching of chart data

Each time the user taps a range tab, useBitcoinMarketChartData fires a fresh fetch. Toggling between "1D" and "1W" repeatedly hammers CoinGecko/Coinbase on every tap.

Suggestion: Cache fetched data per (fiat, windowSecs) key in a ref or module-level Map, at least for the component's lifetime. The data doesn't go stale in seconds.

---

🟡 CoinGecko "All" fallback is misleading

BitcoinDetail.tsx:600 (line 1151 in diff)

function coinGeckoDaysForWindow(windowSecs: number): string[] {
  if (isAllChartWindow(windowSecs)) return ['365']

If Coinbase "All" fails (which it will for non-USD users per the bug above), the CoinGecko fallback requests 365 days but labels the tab "All". Users see ~1 year of data while the UI says "All time." Either fetch max from CoinGecko or label the fallback honestly.

---

🟡 balance passed to BigInt() without guard

BitcoinDetail.tsx:259 (line 809 in diff)

<span className='asset-detail-holding-amount'>{formatAssetAmount(BigInt(balance), 8)}</span>

balance is typed as number from WalletContext. If it's ever NaN, fractional, or Infinity (e.g., during loading or error states), BigInt() throws. The rest of the codebase treats balance as an integer satoshi count, so this is low-risk, but a guard like BigInt(Math.floor(balance || 0)) is cheap insurance.

---

🟢 Minor / Nits

1. 376 lines of CSS in index.css — This is a growing monolith. Not blocking, but consider splitting per-screen CSS or using CSS modules before this file becomes unmanageable.

2. useTokenColor runs once (BitcoinDetail.tsx:355, line 905 in diff) — The useEffect has [token] as dependency, but token is a constant string '--orange-500'. This means it resolves the CSS custom property once on mount. If the user switches themes, the chart color won't update until the component remounts. Consider adding config.theme to the dependency array or listening for theme changes.

3. Test coverage is minimal — The new test only covers navigation to the detail page (clicking the BTC row). There are no tests for BitcoinDetail itself: chart data fetching, range tab switching, delta calculation, action button handlers, error states. The helper functions (calculateDelta, smoothChartData, bucketAveragePoints, coinGeckoDaysForWindow) are pure and easily unit-testable.

---

✅ What looks good

• Clean extraction of SwapComingSoonSheet into a shared component
• Proper AbortController cleanup in the data-fetching effect
• Good accessibility: role="tablist", aria-selected, aria-hidden, aria-label
• prefers-reduced-motion respected throughout
• ResizeObserver feature detection before rendering Liveline
• contain: content on the page root for paint performance
• Navigation registration is clean and follows existing patterns

---

Verdict: Request changes. The convertFiat bug is a crash for all non-USD users on the "All" tab. Fix that, and this is close to shippable.

[ghost]

Follow-up Review — New Commit 2c53873

All six findings from my previous review have been addressed cleanly:

Finding	Fix
🔴 convertFiat crash (non-USD users)	Removed entirely. Coinbase "All" path now gated on fiat === Fiats.USD; non-USD falls through to CoinGecko. No more phantom undefined function call.
🟡 Unbounded parallel Coinbase requests	Sequential await in loop — no more 14-request burst.
🟡 No chart caching	Module-level Map<string, LivelinePoint[]> keyed by fiat:windowSecs.
🟡 CoinGecko "All" returned 365 days	Now returns 'max'.
🟡 BigInt(balance) unguarded	safeBalance via Number.isFinite + Math.floor + Math.max(0, ...).
🟢 useTokenColor theme reactivity	theme added as dependency + parameter.

No new issues introduced. The fix commit is tight — 33 additions, 32 deletions, all in BitcoinDetail.tsx.

Minor note for future: the chart cache has no TTL, so data can go stale if the user stays on the page for a long session. Not blocking — just worth knowing.

✅ Approved. No protocol-critical code touched (this is a pure UI feature — chart, price display, navigation).

[sahilc0]

@bordalix fixed the chart hover drift João flagged. Liveline is now paused while the user is hovering/scrubbing the bitcoin chart, so holding the cursor/thumb at one point keeps the tooltip value fixed instead of advancing with the live time window. Added a regression test for pause-on-hover/resume-on-leave.

[ghost]

Follow-up Review — Commit 1978776

Single-line addition: paused={chartInteracting} on the <Liveline> component, plus a new test.

What changed:

• BitcoinDetail.tsx:208 — passes existing chartInteracting state as paused prop to freeze the chart animation while the user is hovering/scrubbing. Clean use of already-wired pointer event handlers (setChartScrubbing).
• bitcoin-detail.test.tsx — new test covering the pause-on-hover behavior. Mocks Liveline, ResizeObserver, and fetch correctly. Asserts data-paused toggles on pointerEnter/pointerLeave.

Verdict: No protocol-critical code touched. Pure UI polish. The interaction state was already managed; this just threads it to the chart library. Test coverage is appropriate.

✅ LGTM — no issues found in the new commit.

[ghost]

Follow-up Review — Commit 1978776

Single-line addition: paused={chartInteracting} on the <Liveline> component, plus a new test.

What changed:

• BitcoinDetail.tsx:208 — passes existing chartInteracting state as paused prop to freeze the chart animation while the user is hovering/scrubbing. Clean use of already-wired pointer event handlers (setChartScrubbing).
• bitcoin-detail.test.tsx — new test covering the pause-on-hover behavior. Mocks Liveline, ResizeObserver, and fetch correctly. Asserts data-paused toggles on pointerEnter/pointerLeave.

Verdict: No protocol-critical code touched. Pure UI polish. The interaction state was already managed; this just threads it to the chart library. Test coverage is appropriate.

✅ LGTM — no issues found in the new commit.

[ghost]

Follow-up Review — Commits f2eedfd, 8c7ebb6

Two small production fixes and a batch of test hardening. No protocol-critical code touched.

Production changes (both clean) ✅

src/components/AssetCard.tsx:36 — Removed the ticker-as-name special-case (ticker && ticker !== 'BTC' ? ticker : ...). Now consistently uses name || truncatedAssetId || 'Asset'. All seven callsites already pass name, so no display regression.

src/components/TransactionsList.tsx:112 — Added ticker as a fallback in the label chain: accountTicker ?? ticker ?? meta?.name ?? truncated. Correct — for assets where accountTickerForAssetTicker() returns undefined but a raw ticker exists, this prevents falling through to the less-readable metadata name or truncated ID.

Test changes ✅

• Increased flaky timeouts (4000 → 10000ms)
• Made balance assertions deterministic: computes expected balances from totalSent = amount + fees instead of scraping the DOM with regex — much more robust
• Removed prettyNumber formatting from assertions to match actual rendered output
• Fixed selectors for the updated transaction display format (dropped - $ prefix)

No issues found. Ship it.

[ghost]

Follow-up Review — Commits 97f0aa7, 9d782bd, 48cdbad, 732301f

Good refactor: replaces direct Coinbase/CoinGecko calls with a backend worker proxy (fetchHistoricalMarketData). Removes ~100 lines of client-side chunked fetching. Not protocol-critical — this is purely market-data UI. Three findings:

---

🟡 Missing AbortSignal passthrough — requests can't be cancelled

src/lib/marketData.ts:28-33

fetchHistoricalMarketData doesn't accept or forward an AbortSignal. The caller in BitcoinDetail.tsx:384-405 still creates an AbortController and checks signal.aborted after resolution, but the underlying fetch() is never abortable.

If a user rapidly switches chart tabs (1H → 1D → 1W → 1M), all 4 requests fire and complete — none are cancelled. The abort check after .then() prevents stale data from rendering, but the requests themselves are wasted. On slow connections this means unnecessary network load and delayed rendering of the final tab.

Suggested fix:

export const fetchHistoricalMarketData = async (
  secs: number, fiat: Fiats, signal?: AbortSignal
): Promise<LivelinePoint[]> => {
  // ...
  const resp = await fetch(`${host}?${params.toString()}`, { signal })
  // ...
}

---

🟡 No response.ok check — silent failure on worker errors

src/lib/marketData.ts:31-32

const resp = await fetch(`${host}?${params.toString()}`)
const data = await resp.json()

If the worker returns a 500/502/404, resp.json() will either throw on non-JSON bodies (uncaught, bubbles as a generic parse error) or parse an error object that fails validation and silently returns []. The old CoinGecko/Coinbase code explicitly checked response.ok and threw descriptive errors.

Suggested fix:

if (!resp.ok) throw new Error(`Market data fetch failed: ${resp.status}`)

---

🟡 CHART_WINDOWS duplicated — drift risk

src/lib/marketData.ts:12-19 and src/screens/Wallet/BitcoinDetail.tsx:30-37

Both files define their own CHART_WINDOWS array. The secs values must match for secsToPeriod() to return the correct label. If someone later adds or reorders a window in one file but not the other, it silently falls back to 'oneDay'.

Consider exporting the canonical list from marketData.ts and importing it in BitcoinDetail.tsx, or at minimum exporting the secsToPeriod mapping so there's one source of truth for window → API period.

---

Not blocking. Default chart changed from 1H to 1W — makes sense for a detail page. Console.log removed. Worker URL has a TODO to move off personal subdomain (tracked). All clean. The three findings above are quality improvements, not blockers.

[ghost]

Follow-up Review — Commit 1820528

Addresses both findings from my last review:

1. ✅ AbortSignal passthrough — signal?: AbortSignal added to fetchHistoricalMarketData and forwarded to fetch(). Rapid tab-switching now cancels in-flight requests properly.
2. ✅ response.ok check — Worker errors now throw a descriptive Error instead of silently falling through to JSON parse failures.

Rename CHART_WINDOWS → PERIOD_WINDOWS in marketData.ts is a clarity improvement. The remaining duplication between PERIOD_WINDOWS (API labels) and CHART_WINDOWS in BitcoinDetail.tsx (display labels) is fine — they serve different purposes and the secs values match.

No new issues. LGTM.