Skip to content

fix max send in fiat only mode - #214

Merged
bordalix merged 4 commits into
masterfrom
fiat_ui_fix
Nov 12, 2025
Merged

bordalix merged 4 commits into
masterfrom
fiat_ui_fix

Conversation

@bordalix

@bordalix bordalix commented Nov 7, 2025 •

Copy link
Copy Markdown
Collaborator

Fixes bug when clicking MAX button in Send when in fiat only mode.

Summary by CodeRabbit

  • Bug Fixes
    • Improved amount calculations and synchronization between satoshis and fiat.
    • Enhanced validation and error feedback for limits (insufficient funds, min/max, LNURL).
    • Updated "Send" button labels and messages to reflect validation and server reachability; preserves prior errors when server becomes reachable.
    • Refined "Send All" handling with accurate fee-aware amount computation.
    • Better flow progression when recipient, pending swap, and proceed state are present.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Nov 7, 2025 •

Copy link
Copy Markdown

Deploying wallet-signet with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9ad992a
Status: ✅  Deploy successful!
Preview URL: https://9883394f.wallet-23u.pages.dev
Branch Preview URL: https://fiat-ui-fix.wallet-23u.pages.dev

View logs

@coderabbitai

coderabbitai Bot commented Nov 7, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Derive and synchronize satoshi/fiat amounts and UI state in the Send form: new effects compute sats from amount, update validation/errors/button label (including LNURL, min/max, 1-sat, balance), handle Ark server reachability, adjust proceed flow (including submarine-swap) and update send-all behavior.

Changes

Cohort / File(s) Summary
Send form state & effects
src/screens/Wallet/Send/Form.tsx
Added effects to derive satoshis from amount (fiat↔sats normalization), synchronize sendInfo (validation, error text, dynamic button label) against availableBalance, LNURL limits, min/max and 1‑satoshi floor, and Ark server reachability; included pendingSwap in proceed flow and adjusted send‑all to compute sats minus submarine-swap fees then convert to fiat when needed.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User
    participant Form as Send Form
    participant Effects as Derived Effects
    participant Validators as Validators/Rules
    participant Nav as Navigator

    User->>Form: Enter amount / toggle send-all
    Form->>Effects: amount changed
    Effects->>Effects: Normalize amount → compute satoshis (fiat↔sats)
    Effects->>Validators: Validate sats vs availableBalance, LNURL limits, min/max, 1-sat
    Validators-->>Form: Update error text & button label
    Effects->>Effects: Check Ark server reachability → set unreachable error/label if needed
    User->>Form: Click proceed
    Form->>Effects: proceed flag true
    Effects->>Effects: If valid & proceed -> handle pendingSwap/submarine-swap or route to SendDetails
    Effects->>Nav: Navigate to next screen
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay attention to ordering and dependency arrays of effects to avoid races or circular updates.
  • Verify numeric normalization and rounding/flooring for sats and fiat conversions to prevent off-by-one/dust issues.
  • Check send-all computation properly subtracts submarine-swap fees and behaves consistently across fiat/sat modes.
  • Ensure Ark unreachable handling does not inadvertently suppress other validation errors.

Possibly related PRs

Suggested reviewers

  • Kukks

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix max send in fiat only mode' directly addresses the main objective of the PR, which is to fix a bug with the MAX button in Send when in fiat-only mode, as confirmed by the PR description and objectives.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fiat_ui_fix

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f10e62e and 9ad992a.

📒 Files selected for processing (1)
  • src/screens/Wallet/Send/Form.tsx (4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/Send/Form.tsx
⏰ 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: test

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

cloudflare-workers-and-pages Bot commented Nov 7, 2025 •

Copy link
Copy Markdown

Deploying wallet-mutinynet with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9ad992a
Status: ✅  Deploy successful!
Preview URL: https://826ff8a9.arkade-wallet.pages.dev
Branch Preview URL: https://fiat-ui-fix.arkade-wallet.pages.dev

View logs

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Nov 7, 2025 •

Copy link
Copy Markdown

Deploying wallet-bitcoin with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9ad992a
Status: ✅  Deploy successful!
Preview URL: https://f4a9a2b5.wallet-bitcoin.pages.dev
Branch Preview URL: https://fiat-ui-fix.wallet-bitcoin.pages.dev

View logs

@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: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5b32314 and 0fe4f71.

📒 Files selected for processing (1)
  • src/screens/Wallet/Send/Form.tsx (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/Send/Form.tsx

Comment thread src/screens/Wallet/Send/Form.tsx Resolved
Comment thread src/screens/Wallet/Send/Form.tsx Resolved
@theDavidCoen

Copy link
Copy Markdown

@bordalix the fix makes it possible to send max, but the amount in sats is still wrong if you switch to EUR.
See this video

Video.del.2025-11-11.13-43-37.mp4

In this test (with https://ef49b01c.wallet-bitcoin.pages.dev/), I received 1000 sats and sent MAX.
In wallet 2 I correctly received 1000 sats, but in wallet 1 the UI shows 1003 sats.

@theDavidCoen theDavidCoen mentioned this pull request Nov 11, 2025
@theDavidCoen

Copy link
Copy Markdown

pass as for the huddle @bordalix

@bordalix
bordalix merged commit b2393aa into master Nov 12, 2025
1 check passed
@bordalix
bordalix deleted the fiat_ui_fix branch November 26, 2025 14:30
@coderabbitai coderabbitai Bot mentioned this pull request Mar 21, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Apr 8, 2026
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.

2 participants