Skip to content

fix fiat value on receive - #508

Merged
pietro909 merged 1 commit into
masterfrom
fix/fiat_value_on_receive
Apr 1, 2026
Merged

pietro909 merged 1 commit into
masterfrom
fix/fiat_value_on_receive

Conversation

@bordalix

@bordalix bordalix commented Apr 1, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #502

@pietro909 please review

Summary by CodeRabbit

  • Bug Fixes
    • Fixed the amount input component to properly recalculate derived values and error states in edge cases where certain asset information may be incomplete.

@coderabbitai

coderabbitai Bot commented Apr 1, 2026 •

Copy link
Copy Markdown
Contributor

Walkthrough

Modified the useEffect guard condition in InputAmount.tsx from checking asset truthiness to specifically checking asset?.assetId presence. This ensures otherValue and error recalculate when asset is provided but lacks assetId, rather than skipping the update.

Changes

Cohort / File(s) Summary
InputAmount Effect Guard
src/components/InputAmount.tsx
Changed useEffect guard condition from if (asset) return to if (asset?.assetId) return to allow recalculation of otherValue/error when asset lacks assetId.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

  • fix input amount in fiat only #246: Also modifies src/components/InputAmount.tsx useEffect initialization logic, specifically affecting how otherValue/error are derived from sats prop.

Suggested reviewers

  • pietro909
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix fiat value on receive' directly matches the issue being addressed: fixing alternative value updates on the Receive screen.
Linked Issues check ✅ Passed The PR modifies InputAmount.tsx's useEffect guard to check asset?.assetId instead of asset, enabling otherValue recalculation when asset lacks assetId. This addresses issue #502's root cause where alternative values weren't updating due to the useEffect early return condition being too broad.
Out of Scope Changes check ✅ Passed The single-line change to InputAmount.tsx directly addresses the identified root cause in issue #502 with no extraneous modifications.

✏️ 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/fiat_value_on_receive

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

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

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying wallet-bitcoin with  Cloudflare Pages  Cloudflare Pages

Latest commit: 270514a
Status: ✅  Deploy successful!
Preview URL: https://c2a36b49.wallet-bitcoin.pages.dev
Branch Preview URL: https://fix-fiat-value-on-receive.wallet-bitcoin.pages.dev

View logs

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying wallet-mutinynet with  Cloudflare Pages  Cloudflare Pages

Latest commit: 270514a
Status: ✅  Deploy successful!
Preview URL: https://c2768dd5.arkade-wallet.pages.dev
Branch Preview URL: https://fix-fiat-value-on-receive.arkade-wallet.pages.dev

View logs

@ghost

ghost commented Apr 1, 2026

Copy link
Copy Markdown

🔍 Arkana PR Review — wallet#508

fix fiat value on receive | 1 file changed | +1 / -1

Summary

One-line fix in InputAmount.tsx: changes the early return guard from if (asset) return to if (asset?.assetId) return.

Analysis

Root cause: The previous condition if (asset) return would skip the fiat value update whenever asset was any truthy value — including the default BTC asset object (which may not have an assetId property). This meant the secondary value (fiat equivalent) never updated when receiving BTC.

Fix correctness: ✅ By checking asset?.assetId instead, the guard now only triggers when a non-BTC asset is selected (i.e., an asset with an explicit assetId). For BTC receives, the useEffect correctly runs and updates the fiat display.

Security

No security concerns — this is a UI display fix only. No transaction logic, key handling, or VTXO operations are affected.

Notes

LGTM ✅

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/InputAmount.tsx`:
- Around line 62-65: The effect that updates otherValue and error (calls
setOtherValue and setError) depends on useFiat, toFiat, and fiatDecimals but
only lists sats in its dependency array, so toggling fiat/sats leaves otherValue
stale; update the dependency array for that useEffect (the block that checks
asset?.assetId and calls setOtherValue(setError)) to include sats, useFiat,
toFiat, and fiatDecimals, or alternatively stabilize toFiat/fiatDecimals via
refs before using them in the effect.
🪄 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: 75e9b11a-f575-4d72-a1e9-8c84a778fe53

📥 Commits

Reviewing files that changed from the base of the PR and between abb8e4f and 270514a.

📒 Files selected for processing (1)
  • src/components/InputAmount.tsx

Comment thread src/components/InputAmount.tsx Resolved
@pietro909
pietro909 self-requested a review April 1, 2026 10:38
@pietro909
pietro909 merged commit 6057366 into master Apr 1, 2026
5 of 6 checks passed
@pietro909
pietro909 deleted the fix/fiat_value_on_receive branch April 1, 2026 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Receive is not updating alternative value

2 participants