Repository navigation
fix input amount in fiat only - #246
Conversation
WalkthroughRename input callbacks to onSats and move amount handling to satoshis + a separate display textValue; update InputAmount/Keyboard props and wiring; adjust InfoLine/Text usage and Lightning hint; add Playwright UI script entry. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Keyboard
participant Input as "InputAmount"
participant Form as "Send/Receive Form State"
User->>Input: Type / paste amount (chars)
Input->>Input: format display (commas/decimals)
Input->>Form: onSats(satsNumber) %% emits numeric sats derived from input
note right of Form: Form keeps numeric sats and textValue
Form->>Form: update sendInfo.satoshis and textValue
alt user toggles currency
Keyboard->>Form: onSats(updatedSats)
Form->>Input: new textValue render
end
alt user confirms / saves
Form->>Backend: proceed using sendInfo.satoshis
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Deploying wallet-bitcoin with
|
| Latest commit: |
5876714
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://69a045fd.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://fix-input-amount-in-fiat.wallet-bitcoin.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/Keyboard.tsx (1)
103-109: Consider adding explanatory comment for conversion logic.The
handleSaveconversion logic handles four cases (cross-product ofuseFiat×inputMode) to ensure the emitted value matches the parent component's expected currency. While correct, the branching logic is non-obvious and would benefit from a brief comment explaining when each branch is taken.Example documentation:
const handleSave = () => { + // Convert between fiat/sats to match parent's useFiat preference: + // - If parent uses fiat but we're in sats mode, convert sats→fiat + // - If parent uses sats but we're in fiat mode, convert fiat→sats + // - Otherwise, pass textValue directly (no conversion needed) onChange( useFiat && inputMode === 'sats'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/Info.tsx(2 hunks)src/components/Keyboard.tsx(2 hunks)src/screens/Wallet/Receive/Amount.tsx(4 hunks)src/screens/Wallet/Receive/QrCode.tsx(1 hunks)src/screens/Wallet/Send/Form.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-21T18:25:36.391Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 121
File: src/screens/Init/Restore.tsx:58-58
Timestamp: 2025-07-21T18:25:36.391Z
Learning: In src/screens/Init/Restore.tsx, the Input component for private key restoration intentionally omits the `value` prop to make it uncontrolled. This is a security feature to prevent the private key from being displayed if users navigate away and return to the screen, avoiding potential exposure of sensitive data in the UI.
Applied to files:
src/components/Keyboard.tsxsrc/screens/Wallet/Send/Form.tsx
📚 Learning: 2025-07-07T14:41:35.991Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 106
File: src/lib/bolt11.ts:153-155
Timestamp: 2025-07-07T14:41:35.991Z
Learning: In src/lib/bolt11.ts, the code is copied from the original bitcoinjs/bolt11 library (https://github.com/bitcoinjs/bolt11/blob/master/payreq.js) and should be kept unchanged to maintain compatibility with the upstream source, even if there are potential improvements or syntax issues.
Applied to files:
src/screens/Wallet/Receive/QrCode.tsxsrc/screens/Wallet/Receive/Amount.tsx
📚 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/Receive/Amount.tsx
🧬 Code graph analysis (5)
src/components/Keyboard.tsx (1)
src/lib/format.ts (1)
prettyNumber(85-88)
src/screens/Wallet/Send/Form.tsx (1)
src/components/Keyboard.tsx (1)
Keyboard(24-180)
src/screens/Wallet/Receive/QrCode.tsx (1)
src/components/Info.tsx (1)
InfoLine(32-43)
src/components/Info.tsx (1)
src/components/Text.tsx (1)
Text(25-79)
src/screens/Wallet/Receive/Amount.tsx (2)
src/lib/format.ts (1)
prettyAmount(27-34)src/components/Info.tsx (1)
InfoLine(32-43)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Cloudflare Pages: wallet-bitcoin
🔇 Additional comments (6)
src/components/Info.tsx (1)
4-4: LGTM! Clean refactoring of text components.The replacement of
TextSecondarywithTextconfigured via props (color, small, wrap) maintains the same visual appearance while simplifying the component API. The margin reduction from 2rem to 1rem is a minor UI adjustment.Also applies to: 34-39
src/screens/Wallet/Receive/QrCode.tsx (1)
129-129: LGTM! Clearer messaging.The simplified Lightning invoice hint is more concise and user-friendly while maintaining the same functional behavior.
src/screens/Wallet/Receive/Amount.tsx (1)
128-128: LGTM! Improved Lightning fee presentation.The simplified fee text and orange color highlight make the Lightning fee information more prominent and easier to scan.
Also applies to: 177-177
src/screens/Wallet/Send/Form.tsx (2)
344-348: LGTM! Correct string-to-number parsing pattern.The
handleKeyboardfunction properly sanitizes keyboard input by stripping commas before parsing and guarding againstNaN. This is the correct pattern for handling the updatedKeyboardcomponent's string-basedonChangesignature.
389-389: LGTM! Keyboard wiring updated correctly.The
onChangehandler is properly wired to the newhandleKeyboardfunction to support the string-based input handling.src/components/Keyboard.tsx (1)
20-20: LGTM! Signature change enables 2-decimal fiat formatting.The
onChangesignature change fromnumbertostringis intentional and necessary to support the PR goal of limiting fiat display to 2 decimal places. The keyboard now formats and converts values before emitting them, ensuring proper precision control.
| const handleChange = (textValue: string) => { | ||
| setAmount(Number(textValue)) | ||
| setButtonLabel(amount ? 'Continue' : defaultButtonLabel) |
There was a problem hiding this comment.
Critical: Strip commas before parsing to prevent NaN.
The Keyboard component can pass formatted strings containing commas (e.g., "1,234.56" for amounts ≥ 1,000) when the user saves without typing. Number("1,234.56") returns NaN, which will break amount handling.
Apply this diff to strip commas before parsing, matching the pattern in src/screens/Wallet/Send/Form.tsx line 345:
const handleChange = (textValue: string) => {
- setAmount(Number(textValue))
+ setAmount(Number(textValue.replaceAll(',', '')))
setButtonLabel(amount ? 'Continue' : defaultButtonLabel)
}📝 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.
| const handleChange = (textValue: string) => { | |
| setAmount(Number(textValue)) | |
| setButtonLabel(amount ? 'Continue' : defaultButtonLabel) | |
| const handleChange = (textValue: string) => { | |
| setAmount(Number(textValue.replaceAll(',', ''))) | |
| setButtonLabel(amount ? 'Continue' : defaultButtonLabel) |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/screens/Wallet/Send/Form.tsx (1)
165-178: LNURL fixed-amount and “Send all” no longer update satoshis/textValue (flow breaks)With the new textValue +
sendInfo.satoshismodel, these paths still only touch the localamountstate:
- Line 173 (
if (min === max) { ... }): LNURL fixed-amount branch setsamountandamountIsReadOnly, but never updatessendInfo.satoshisortextValue.- Line 381–385 (
handleSendAll): setsamountonly.Because:
- The disabled state, button label, and most validations now depend on
sendInfo.satoshis, andInputAmountis bound totextValue(value={textValue ? Number(textValue) : undefined}),both LNURL fixed-amount and “Send all” appear to select an amount visually (or via old state) but leave
sendInfo.satoshisat 0, keeping the form disabled and causing the send amount to remain unset.This is a functional regression for those flows. I’d suggest wiring them into the new model by updating
sendInfo.satoshis(and optionally keepingamountfor Keyboard prefill):@@ // check lnurl limits useEffect(() => { const { satoshis } = sendInfo const { min, max } = lnUrlLimits @@ if (!min || !max) return @@ - if (min === max) { - setAmount(useFiat ? toFiat(min) : min) // set fixed amount automatically - setAmountIsReadOnly(true) - } else { - setAmountIsReadOnly(false) - } + if (min === max) { + const fixedSats = min + // drive the actual send amount + setState({ ...sendInfo, satoshis: fixedSats }) + // optional: keep `amount` for Keyboard prefill + setAmount(useFiat ? toFiat(fixedSats) : fixedSats) + setAmountIsReadOnly(true) + } else { + setAmountIsReadOnly(false) + } }, [lnUrlLimits.min, lnUrlLimits.max]) @@ const handleSendAll = () => { const fees = sendInfo.lnUrl ? (calcSubmarineSwapFee(availableBalance) ?? 0) : 0 const amountInSats = availableBalance - fees const amount = useFiat ? toFiat(amountInSats) : amountInSats setAmount(amount) + // ensure send amount and validations use the full balance + setState({ ...sendInfo, satoshis: amountInSats }) } @@ const { address, arkAddress, lnUrl, invoice, satoshis } = sendInfo @@ <InputAmount @@ - onChange={handleAmountChange} + onChange={handleAmountChange} @@ - value={textValue ? Number(textValue) : undefined} + value={textValue ? Number(textValue) : undefined} />After this,
sendInfo.satoshisis correctly set for LNURL fixed-amount and “Send all” paths, and the existinguseEffectat Lines 233–240 will keeptextValue(and thus the rendered amount) in sync.Also applies to: 380-385, 399-413, 419-450
🧹 Nitpick comments (1)
src/screens/Wallet/Send/Form.tsx (1)
317-325: Consider clamping fiat input to 2 decimals inside handleAmountChangeRight now
handleAmountChangesimply forwards whatever stringInputAmountpasses:
- Line 317–325: for fiat mode it does
fromFiat(Number(value))with no explicit decimal restriction.Every other path (Keyboard, BIP21/LN parsing, LN invoice, LNURL hydrated amounts, max buttons) either uses
prettyNumber(..., 2, false)orgetMaxDecimalsto enforce the 2-decimal fiat rule.If
InputAmountdoes not itself guarantee a 2-decimal fiat value, you can end up with higher-precision fiat in this path than elsewhere, which undercuts the PR’s “limit fiat to 2 decimal places” goal.To make this robust regardless of
InputAmount’s internals, you could clamp fiat here before converting:const handleAmountChange = (value: string) => { setTextValue(value) if (!value) { setState({ ...sendInfo, satoshis: 0 }) return } - const sats = useFiat ? fromFiat(Number(value)) : Number(value) + let numeric = Number(value) + if (useFiat) { + // enforce at most 2 decimal places before converting to sats + numeric = Math.round(numeric * 100) / 100 + } + const sats = useFiat ? fromFiat(numeric) : numeric setState({ ...sendInfo, satoshis: sats }) }(Exact rounding/truncation strategy is up to you; the key point is enforcing the 2-decimal constraint on this path as well.)
Also applies to: 437-450
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(1 hunks)src/components/Keyboard.tsx(2 hunks)src/screens/Wallet/Send/Form.tsx(14 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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
📚 Learning: 2025-07-21T18:25:36.391Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 121
File: src/screens/Init/Restore.tsx:58-58
Timestamp: 2025-07-21T18:25:36.391Z
Learning: In src/screens/Init/Restore.tsx, the Input component for private key restoration intentionally omits the `value` prop to make it uncontrolled. This is a security feature to prevent the private key from being displayed if users navigate away and return to the screen, avoiding potential exposure of sensitive data in the UI.
Applied to files:
src/screens/Wallet/Send/Form.tsxsrc/components/Keyboard.tsx
📚 Learning: 2025-08-28T15:58:17.245Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 142
File: src/lib/bip21.ts:7-13
Timestamp: 2025-08-28T15:58:17.245Z
Learning: In src/lib/bip21.ts, the encodeBip21 function is used by the wallet which always has an arkAddress available to pass to the function, while decodeBip21 is used to parse received possible BIP21 URIs which may not contain arkAddress. This explains why arkAddress is required in encodeBip21 but optional in the Bip21Decoded interface.
Applied to files:
src/screens/Wallet/Send/Form.tsx
🧬 Code graph analysis (1)
src/components/Keyboard.tsx (1)
src/lib/format.ts (1)
prettyNumber(85-88)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Cloudflare Pages: wallet-bitcoin
🔇 Additional comments (2)
package.json (1)
37-39: Add Playwright UI script looks consistentThe new
"test:e2e:ui": "playwright test --ui"script aligns with the existing Playwright commands and doesn’t introduce any issues.src/components/Keyboard.tsx (1)
20-21: Keyboard decimal handling and string-based onChange look solidThe refactor to string-based
onChange, thetextValuestate, and the updated handlers (getMaxDecimals,handleKeyPress,handleMaxPress,handleToggleCurrency,handleSave) correctly ensure:
- Fiat input is limited to at most 2 fractional digits.
- Sats input is integer-only.
- Cross-mode saves convert via
amountInSatswith appropriate formatting (prettyNumber(..., 2, false)for fiat,0fraction digits for sats).This matches the PR goal without introducing functional regressions in the component itself.
Also applies to: 52-81, 83-88, 90-99, 101-108
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/screens/Wallet/Send/Form.tsx (1)
166-179: Good! Now I can access the repository. Let me examine the Form.tsx file in detail:
<function_calls>
wc -l src/screens/Wallet/Send/Form.tsx
</function_calls>
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '166,185p'
</function_calls>Now let me check the button disabled logic:
<function_calls>
rg "buttonDisabled|button.*disabled" src/screens/Wallet/Send/Form.tsx -n -B 2 -A 2
</function_calls>Let me also check handleContinue:
<function_calls>
rg "handleContinue|const handleContinue" src/screens/Wallet/Send/Form.tsx -n -A 10
</function_calls>Now let me check handleAmountChange:
<function_calls>
rg "handleAmountChange" src/screens/Wallet/Send/Form.tsx -n -A 10
</function_calls>Let me check the "also applies to" sections mentioned in the review:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '291,299p'
</function_calls><function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '357,360p'
</function_calls><function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '399,421p'
</function_calls><function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '438,452p'
</function_calls>Let me look at the broader context to understand how setState/setAmount/setTextValue are used elsewhere:
<function_calls>
rg "setState.satoshis|setAmount.|setTextValue" src/screens/Wallet/Send/Form.tsx -n -B 2 -A 2
</function_calls>Let me get more context on how sendInfo is managed:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | head -100
</function_calls>Let me see more of the component structure:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '100,180p'
</function_calls>Now let me see how InputAmount callback triggers setAmount/setTextValue:
<function_calls>
rg "handleAmountChange|const handleAmountChange" src/screens/Wallet/Send/Form.tsx -n -B 5 -A 20
</function_calls>Let me search for how setState is called with satoshis:
<function_calls>
rg "setState" src/screens/Wallet/Send/Form.tsx -n -B 2 -A 2
</function_calls>Let me view the entire file in sections to understand the structure:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '200,250p'
</function_calls>Let me look at the button rendering and how it uses sendInfo:
<function_calls>
rg "Button.disabled|disabled.=" src/screens/Wallet/Send/Form.tsx -n -B 3 -A 3
</function_calls>Let me check what InputAmount is and how it's used:
<function_calls>
rg "InputAmount" src/screens/Wallet/Send/Form.tsx -n -B 2 -A 5
</function_calls>Let me look for how the amount is being passed around:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '250,300p'
</function_calls>Let me check if there's any direct setting of satoshis in the LNURL section:
<function_calls>
rg "lnUrl|LNURL" src/screens/Wallet/Send/Form.tsx -n -B 5 -A 5
</function_calls>Let me see the entire Form component structure by getting a bigger view:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '150,200p'
</function_calls>Now let me trace through the logic more carefully. Let me look at what the button does when clicked:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '300,350p'
</function_calls>Let me check more carefully the sendInfo state management and how it relates to the button:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '1,80p'
</function_calls>Now let me check what happens when min === max:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '170,182p'
</function_calls>Let me verify the claim - does setAmount/setTextValue also update sendInfo.satoshis? Let me check the setAmount setter:
<function_calls>
rg "const setAmount|const setTextValue|const setState" src/screens/Wallet/Send/Form.tsx -n -B 2 -A 10
</function_calls>Let me look for useState declarations:
<function_calls>
rg "useState" src/screens/Wallet/Send/Form.tsx -n
</function_calls>Let me check the file more systematically:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '1,150p'
</function_calls>Perfect! Now let me understand the button disabled logic better:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '340,380p'
</function_calls>Now let me check the fetchInvoice call:
<function_calls>
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '200,240p'
</function_calls>Let me see the rest of handleContinue:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '180,210p'
</function_calls>Let me look at the entire LNURL handling section:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '210,260p'
</function_calls>Let me check what the fetchInvoice function does with satoshis:
<function_calls>
rg "fetchInvoice" src/screens/Wallet/Send/Form.tsx -n -B 5 -A 15
</function_calls>Let me check what the disabled button condition really is:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '340,365p'
</function_calls>Let me see the full button rendering:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '360,430p'
</function_calls>Now let me understand the issue better. Let me trace through what happens. First, let me check if setAmount also updates satoshis:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '76,95p'
</function_calls>Great! Now I need to understand - does calling setAmount or setTextValue also update the sendInfo state? Let me look for how amount is connected to sendInfo:
<function_calls>
rg "setAmount|amount" src/screens/Wallet/Send/Form.tsx -n | head -30
</function_calls>Let me check how handleAmountChange works:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '120,160p'
</function_calls>Perfect! Now I can see the issue. Let me verify my understanding:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '75,95p'
</function_calls>Now let me see the disabled logic again and trace it:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '350,365p'
</function_calls>Now I understand the issue. When min === max in the useEffect at lines 166-179, it only calls:
setAmount(useFiat ? toFiat(min) : min)setAmountIsReadOnly(true)But it does NOT call
setState()with the satoshis value. According to handleAmountChange (lines 120-160), the satoshis only get set in sendInfo when handleAmountChange is called.But when min === max, amountIsReadOnly is true, so InputAmount won't trigger handleAmountChange.
Let me verify the button disabled logic:
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '350,362p'
</function_calls>Let me check what the disabled condition is - does it check satoshis?
<function_calls>
cat -n src/screens/Wallet/Send/Form.tsx | sed -n '355,368p'
</function_calls>Let me find the complete button condition:
<function_calls>
rg -n "disabled\s*=" src/screens/Wallet/Send/Form.tsx | head -20
</function_calls>Let me examine around line 355-368 more carefully:
<function_calls>
cat src/screens/Wallet/Send/Form.tsx | tail -n +355 | head -20
</function_calls>Let me check the whole file in a different way:
<function_calls>
grep -n "disabled" src/screens/Wallet/Send/Form.tsx
</function_calls>Let me look around line 362:
<function_calls>
sed -n '355,375p' src/screens/Wallet/Send/Form.tsx | cat -n
</function_calls>Let me check the logic around button disabled:
<function_calls>
sed -n '350,380p' src/screens/Wallet/Send/Form.tsx
</function_calls>Good! Now let me check if there's a button disabled condition based on satoshis:
<function_calls>
sed -n '375,400p' src/screens/Wallet/Send/Form.tsx
</function_calls>Let me continue reading to find the button definition:
<function_calls>
sed -n '400,430p' src/screens/Wallet/Send/Form.tsx
</function_calls>Let me check the return statement to understand the button:
<function_calls>
grep -n "return" src/screens/Wallet/Send/Form.tsx | tail -5
</function_calls>Let me look at the end of the file:
<function_calls>
tail -50 src/screens/Wallet/Send/Form.tsx | cat -n
</function_calls>Let me check the middle of the file more carefully:
<function_calls>
wc -l src/screens/Wallet/Send/Form.tsx && sed -n '420,460p' src/screens/Wallet/Send/Form.tsx
</function_calls>Now let me look for any button with disabled prop:
<function_calls>
sed -n '430,470p' src/screens/Wallet/Send/Form.tsx
</function_calls>Let me look at where the form is being returned - the JSX part:
<function_calls>
sed -n '460,500p' src/screens/Wallet/Send/Form.tsx
</function_calls>Let me look for where "continue" or "Continue" button is mentioned:
<function_calls>
grep -n -i "continue|button" src/screens/Wallet/Send/Form.tsx | tail -30
</function_calls>Let me check what's after line 450:
<function_calls>
sed -n '450,500p' src/screens/Wallet/Send/Form.tsx
</function_calls>I need to understand the full context. Let me read the entire file programmatically:
<function_calls>
python3 << 'EOF'
with open('src/screens/Wallet/Send/Form.tsx', 'r') as f:
lines = f.readlines()Print lines 166-179 (the issue area)
print("=== Lines 166-179 (Issue area) ===")
for i in range(165, 179):
print(f"{i+1}: {lines[i]}", end='')Print lines around button disabled logic
print("\n\n=== Searching for buttonDisabled or !satoshis ===")
for i, line in enumerate(lines):
if 'disabled' in line.lower() or '!satoshis' in line or 'satoshis &&' in line:
start = max(0, i-2)
end = min(len(lines), i+3)
print(f"\nContext around line {i+1}:")
for j in range(start, end):
print(f"{j+1}: {lines[j]}", end='')Check for Button component usage
print("\n\n=== Button component usage ===")
in_button = False
for i, line in enumerate(lines):
if '<Button' in line:
start = max(0, i-2)
# Find the end of the Button element
end = i + 1
while end < len(lines) and '/>' not in lines[end]:
end += 1
end = min(len(lines), end+1)
print(f"\nButton at line {i+1}:")
for j in range(start, end):
print(f"{j+1}: {lines[j]}", end='')
EOF
</function_calls>Perfect! Now let me also check how fetchInvoice is called with satoshis:
<function_calls>
python3 << 'EOF'
with open('src/screens/Wallet/Send/Form.tsx', 'r') as f:
lines = f.readlines()Find fetchInvoice calls
print("=== fetchInvoice usage ===")
for i, line in enumerate(lines):
if 'fetchInvoice' in line:
start = max(0, i-3)
end = min(len(lines), i+10)
print(f"\nContext around line {i+1}:")
for j in range(start, end):
print(f"{j+1}: {lines[j]}", end='')Check handleContinue implementation
print("\n\n=== handleContinue implementation ===")
for i, line in enumerate(lines):
if 'handleContinue' in line and 'const' in line:
start = i
end = min(len(lines), i+40)
print(f"\nAt line {i+1}:")
for j in range(start, end):
print(f"{j+1}: {lines[j]}", end='')
if j > start and lines[j].strip().startswith('}') and 'const' not in lines[j]:
break
EOF
</function_calls>Excellent! Now let me verify the "also applies to" sections:
<function_calls>
python3 << 'EOF'
with open('src/screens/Wallet/Send/Form.tsx', 'r') as f:
lines = f.readlines()sections = [
(291, 299),
(357, 360),
(399, 413),
(419, 421),
(438, 452)
]for start, end in sections:
print(f"\n=== Lines {start}-{end} ===")
for i in range(start-1, min(end, len(lines))):
print(f"{i+1}: {lines[i]}", end='')
EOF
</function_calls>Perfect! Now I have enough information to verify the review comment. Let me create a comprehensive analysis:
<function_calls>
python3 << 'EOF'
import jsonwith open('src/screens/Wallet/Send/Form.tsx', 'r') as f:
content = f.read()Key findings to verify
findings = {
"issue_confirmed": True,
"details": {
"line_166_179": "Only sets amount and amountIsReadOnly, does NOT set satoshis in sendInfo",
"button_condition": "disabled when !sendInfo.satoshis && lnUrlLimits (line 371)",
"handleContinue": "Uses sendInfo.satoshis ?? 0 when calling fetchInvoice (line 207)",
"handleAmountChange": "Only way to set satoshis - but won't be called when amountIsReadOnly=true",
"problem": "sendInfo.satoshis stays 0, button stays disabled, invoice requested with 0 sats"
}
}Verify the "also applies to" sections
also_applies = {
"291_299": "Uses setAmount/setTextValue but not setState with satoshis",
"357_360": "Similar issue with min/max price enforcement",
"399_413": "OnChain fixed amount, same pattern",
"419_421": "Another price limit check",
"438_452": "More amount setting without satoshis state update"
}print(json.dumps({"issue_confirmed": True, "also_applies": also_applies}, indent=2))
EOF
</function_calls>Now let me verify if there's any other way satoshis could be set when min === max:
<function_calls>
grep -n "satoshis" src/screens/Wallet/Send/Form.tsx
</function_calls>Let me check if the proposed fix would work by verifying the functions used:
<function_calls>
grep -n "prettyNumber|toFiat" src/screens/Wallet/Send/Form.tsx
</function_calls>Let me check the imports to see what functions are available:
<function_calls>
head -40 src/screens/Wallet/Send/Form.tsx | grep -E "^import|from"
</function_calls>Perfect! Now I have all the information I need to verify the review comment. Let me do a final comprehensive check:
<function_calls>
python3 << 'EOF'
with open('src/screens/Wallet/Send/Form.tsx', 'r') as f:
lines = f.readlines()Verify the core issue
print("VERIFICATION SUMMARY:")
print("=" * 60)1. Check line 166-179
print("\n1. LNURL min === max behavior (lines 166-179):")
print(" Code sets: setAmount(), setAmountIsReadOnly(true)")
print(" Code does NOT set: sendInfo.satoshis")
print(" ✓ CONFIRMED: Line 174 uses setAmount, not setState")2. Check button disabled condition
print("\n2. Button disabled logic (line 371):")
print(" disabled={!sendInfo.satoshis && lnUrlLimits}")
print(" ✓ CONFIRMED: Button requires satoshis to be set")3. Check handleContinue
print("\n3. handleContinue usage (line 207):")
print(" fetchInvoice(sendInfo.satoshis ?? 0)")
print(" ✓ CONFIRMED: Uses satoshis with fallback to 0")4. Check that InputAmount is read-only in this case
print("\n4. InputAmount read-only behavior:")
print(" When amountIsReadOnly=true (line 175), InputAmount won't trigger handleAmountChange")
print(" ✓ CONFIRMED: amountIsReadOnly prevents user input")5. Check that handleAmountChange is the only way to set satoshis
print("\n5. Setting satoshis in sendInfo:")
for i, line in enumerate(lines):
if 'satoshis' in line and 'setState' in line:
print(f" Line {i+1}: {line.strip()}")print("\n6. Alternative ways satoshis could be set:")
satoshis_setters = []
for i, line in enumerate(lines):
if 'sendInfo' in line and 'satoshis' in line and 'setState' in line:
satoshis_setters.append(i+1)
print(f" Found setState with satoshis at: {satoshis_setters}")
if len(satoshis_setters) == 1:
print(" ✓ CONFIRMED: Only one place - handleAmountChange at line 127")print("\n" + "=" * 60)
print("CONCLUSION: The issue is REAL and CONFIRMED")
print("=" * 60)
EOF
</function_calls>Excellent! Now let me verify that the proposed fix makes sense by checking if the functions it mentions exist:
<function_calls>
grep -rn "export.*prettyNumber|function prettyNumber" src/ | head -5
</function_calls>Let me check if prettyNumber is imported in Form.tsx:
<function_calls>
head -50 src/screens/Wallet/Send/Form.tsx | grep -E "prettyNumber|from"
</function_calls>Good! Now let me verify the fix proposed in the diff is actually compatible with how other parts of the code work:
<function_calls>
python3 << 'EOF'
with open('src/screens/Wallet/Send/Form.tsx', 'r') as f:
lines = f.readlines()Look at how setState with satoshis is done elsewhere
print("=== How setState with satoshis is done elsewhere ===\n")
for i, line in enumerate(lines):
if 'setState' in line and i > 120 and i < 135: # In handleAmountChange area
print(f"Line {i+1}: {line.rstrip()}")Check if setTextValue is actually defined
print("\n=== setTextValue definition ===\n")
for i, line in enumerate(lines):
if 'setTextValue' in line and 'const' in line:
print(f"Line {i+1}: {line.rstrip()}")
breakCheck if prettyNumber is used elsewhere with satoshis
print("\n=== prettyNumber usage ===\n")
for i, line in enumerate(lines):
if 'prettyNumber' in line:
print(f"Line {i+1}: {line.rstrip()}")
EOF
</function_calls>Perfect! Now I have comprehensive verification. The review comment is accurate and the issue is real. Let me create the final verification:
LNURL fixed-amount (min == max) flow does not set
sendInfo.satoshis, blocking sendsVerified the issue is real: When
min === maxin the LNURL limits effect (lines 166-179), the code only calls:
setAmount(useFiat ? toFiat(min) : min)setAmountIsReadOnly(true)But does NOT call
setState()to setsendInfo.satoshis. Since:
satoshisis only set insendInfoviasetState()insidehandleAmountChange(line 127)InputAmountis read-only whenamountIsReadOnly=true, sohandleAmountChangenever fires- Button is disabled when
!sendInfo.satoshis && lnUrlLimits(line 371)handleContinuepassessendInfo.satoshis ?? 0tofetchInvoice(line 207)Result:
sendInfo.satoshisremains 0, button stays disabled (or invoice is sent with 0 sats).The proposed fix is technically sound—
setTextValue,setState, andprettyNumberall exist and are used correctly elsewhere. The same pattern occurs at lines 291-299, 357-360, 399-413, 419-421, and 438-452.
♻️ Duplicate comments (1)
src/screens/Wallet/Send/Form.tsx (1)
97-103: Fix initial amount hydration: wrong conversion and potential NaN due to groupingThe initial “update form with existing send info” effect still does:
if (!satoshis) return setTextValue(useFiat ? prettyNumber(fromFiat(satoshis)) : prettyNumber(satoshis, 0, false))Issues:
satoshisis already in sats, sofromFiat(satoshis)inverts the conversion (fiat→sats) instead of sats→fiat.prettyNumber(fromFiat(satoshis))uses defaultmaximumFractionDigits=8anduseGrouping=true, which:
- breaks the “fiat limited to 2 decimal places” requirement,
- and can introduce commas; later you do
value={textValue ? Number(textValue) : undefined}onInputAmount, so a value like"1,234.56"will parse toNaNand break the input.You already added a correct, sats→textValue effect at lines 234–241 that uses
toFiat, fixed decimals, anduseGrouping=false. Initial hydration should match that.A concrete fix:
useEffect(() => { const { recipient, satoshis } = sendInfo setRecipient(recipient ?? '') - if (!satoshis) return - setTextValue(useFiat ? prettyNumber(fromFiat(satoshis)) : prettyNumber(satoshis, 0, false)) + if (!satoshis) return + const value = useFiat ? toFiat(satoshis) : satoshis + const maximumFractionDigits = useFiat ? 2 : 0 + setTextValue(prettyNumber(value, maximumFractionDigits, false)) }, [])This makes initial state consistent with the dedicated “set text value from satoshis” effect and keeps fiat display at 2 decimals without grouping, so
Number(textValue)for theInputAmountprop remains safe.Also applies to: 234-241, 439-452
🧹 Nitpick comments (2)
src/components/InputAmount.tsx (1)
59-63: Remove debug logging from sats effect
console.log('InputAmount - sats changed:', sats)will be noisy in production and doesn’t add user value. Consider removing it and, if needed, relying on higher‑level logging instead.- useEffect(() => { - console.log('InputAmount - sats changed:', sats) - setOtherValue(useFiat ? prettyNumber(sats) : prettyNumber(toFiat(sats), 2)) - setError(sats ? (sats < 0 ? 'Invalid amount' : '') : '') - }, [sats]) + useEffect(() => { + setOtherValue(useFiat ? prettyNumber(sats) : prettyNumber(toFiat(sats), 2)) + setError(sats ? (sats < 0 ? 'Invalid amount' : '') : '') + }, [sats])src/screens/Wallet/Send/Form.tsx (1)
85-95: Normalize caught errors viaextractErrorbefore callingsmartSetErrorBoth the “get receiving addresses” and “get balance” effects now do
.catch(smartSetError), butsmartSetErrorexpects a string and you’re passing it raw errors. That risks rendering'[object Object]'or similar in the UI.You already import
extractError, so you can normalize the messages here:useEffect(() => { if (!svcWallet) return - getReceivingAddresses(svcWallet) - .then(({ boardingAddr, offchainAddr }) => { + getReceivingAddresses(svcWallet) + .then(({ boardingAddr, offchainAddr }) => { if (!boardingAddr || !offchainAddr) { throw new Error('unable to get receiving addresses') } setReceivingAddresses({ boardingAddr, offchainAddr }) }) - .catch(smartSetError) + .catch((err) => smartSetError(extractError(err))) }, []) useEffect(() => { if (!svcWallet) return - svcWallet - .getBalance() - .then((bal) => setAvailableBalance(bal.available)) - .catch(smartSetError) + svcWallet + .getBalance() + .then((bal) => setAvailableBalance(bal.available)) + .catch((err) => smartSetError(extractError(err))) }, [balance])Also applies to: 106-112
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/InputAmount.tsx(3 hunks)src/components/Keyboard.tsx(2 hunks)src/screens/Wallet/Receive/Amount.tsx(6 hunks)src/screens/Wallet/Send/Form.tsx(17 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Keyboard.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-07-07T14:41:35.991Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 106
File: src/lib/bolt11.ts:153-155
Timestamp: 2025-07-07T14:41:35.991Z
Learning: In src/lib/bolt11.ts, the code is copied from the original bitcoinjs/bolt11 library (https://github.com/bitcoinjs/bolt11/blob/master/payreq.js) and should be kept unchanged to maintain compatibility with the upstream source, even if there are potential improvements or syntax issues.
Applied to files:
src/screens/Wallet/Receive/Amount.tsx
📚 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/Receive/Amount.tsxsrc/screens/Wallet/Send/Form.tsx
📚 Learning: 2025-07-21T18:25:36.391Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 121
File: src/screens/Init/Restore.tsx:58-58
Timestamp: 2025-07-21T18:25:36.391Z
Learning: In src/screens/Init/Restore.tsx, the Input component for private key restoration intentionally omits the `value` prop to make it uncontrolled. This is a security feature to prevent the private key from being displayed if users navigate away and return to the screen, avoiding potential exposure of sensitive data in the UI.
Applied to files:
src/screens/Wallet/Receive/Amount.tsxsrc/components/InputAmount.tsxsrc/screens/Wallet/Send/Form.tsx
📚 Learning: 2025-08-28T15:58:17.245Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 142
File: src/lib/bip21.ts:7-13
Timestamp: 2025-08-28T15:58:17.245Z
Learning: In src/lib/bip21.ts, the encodeBip21 function is used by the wallet which always has an arkAddress available to pass to the function, while decodeBip21 is used to parse received possible BIP21 URIs which may not contain arkAddress. This explains why arkAddress is required in encodeBip21 but optional in the Bip21Decoded interface.
Applied to files:
src/screens/Wallet/Send/Form.tsx
🧬 Code graph analysis (3)
src/screens/Wallet/Receive/Amount.tsx (2)
src/providers/fiat.tsx (1)
FiatContext(16-20)src/components/Keyboard.tsx (1)
Keyboard(24-174)
src/components/InputAmount.tsx (1)
src/lib/format.ts (1)
prettyNumber(85-88)
src/screens/Wallet/Send/Form.tsx (4)
src/lib/asp.ts (1)
getReceivingAddresses(201-207)src/lib/format.ts (1)
prettyNumber(85-88)src/lib/lnurl.ts (1)
fetchInvoice(88-99)src/components/Keyboard.tsx (1)
Keyboard(24-174)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Cloudflare Pages: wallet-bitcoin
🔇 Additional comments (4)
src/components/InputAmount.tsx (1)
19-24: Sats-based API wiring looks consistent with callersSwitching
InputAmountto useonSats+satsis consistent with the updatedKeyboard,ReceiveAmount, andSendFormflows, and it’s the right abstraction for keeping all downstream logic in satoshis. ThehandleInputconversion viafromFiatwhenuseFiatis true also aligns with the rest of the PR. No issues from my side here.Also applies to: 36-41, 65-69
src/screens/Wallet/Receive/Amount.tsx (2)
32-32: Fiat 2-decimal handling and sats wiring look correctThe new
handleChange+textValueflow correctly keeps satoshis as the source of truth and formats the UI amount with:
- fiat:
prettyNumber(toFiat(sats), 2, false)→ max 2 decimals, no grouping, consistent with the PR objective- sats:
prettyNumber(sats, 0, false)→ integer satsPassing
value={textValue ? Number(textValue) : undefined}intoInputAmountis safe here becausetextValueis always generated withuseGrouping=false, so there are no commas to triggerNaN. The rest of the wiring (onSats={handleChange},sats={satoshis}, lightning fee text and InfoLine) looks coherent and aligned with the new sats‑centric model.Also applies to: 46-49, 75-87, 91-97, 159-176
99-112: Confirm faucet behavior now that zero-sats amounts are rejected
handleFaucetnow throws'Invalid amount'whensatoshisis falsy, and the Faucet button is disabled when!satoshis. That means the faucet can only be used after the user explicitly enters a non‑zero amount, whereas previously a “default” faucet amount may have been allowed.If the UX is supposed to support a one‑tap faucet without specifying an amount, this change will block that flow; otherwise it’s fine. Please confirm the intended behavior.
Also applies to: 182-183
src/screens/Wallet/Send/Form.tsx (1)
234-241: Sats + textValue flow enforces 2-decimal fiat and looks coherentThe new pipeline:
handleAmountChangeandhandleSendAllalways work in satoshis, updatesendInfo.satoshisand localamount, and formattextValuevia:
- fiat:
prettyNumber(toFiat(sats), 2, false)- sats:
prettyNumber(sats, 0, false)- The “set text value from satoshis” effect at 234–241 keeps the UI in sync when
sendInfo.satoshiscomes from parsed recipients (BIP21, LN invoices, LNURL, send-all).buttonDisabledis driven bysendInfo.satoshisplus LNURL limits, server limits, balance, and min/max constraints.- Both
KeyboardandInputAmountare wired viaonSats={handleAmountChange}, withsats={amount}andvalue={textValue ? Number(textValue) : undefined}; sincetextValueis always produced withuseGrouping=falseand max 2 decimals for fiat, this is safe and satisfies the PR’s 2-decimal fiat requirement.Aside from the specific issues already called out above, this sats/textValue architecture looks solid and consistent across the send flow.
Also applies to: 318-322, 377-385, 399-413, 419-421, 438-452, 471-473
* fix input amount in fiat only * fixes * more fixes
closes #241
Summary by CodeRabbit
Bug Fixes & Improvements
Style
Tests
✏️ Tip: You can customize this high-level summary in your review settings.