From 40aef1ea2e38315358dedf3a16618860d7504091 Mon Sep 17 00:00:00 2001 From: "Abdulmalik A." Date: Fri, 28 Aug 2026 18:02:57 +0100 Subject: [PATCH 1/4] feat(send): validate destination address, amount minimum, and balance before submit Adds client-side validation to the send form so bad input never reaches the API (Issue #420): - Extracts the Stellar public-key regex that already existed in WalletAddressInput into a shared lib/validation/stellar-address.ts utility, and reuses it in ConnectStep (reject a malformed address returned by Freighter or restored from localStorage before it can become the funding/recovery address) and WalletAddressInput itself. - DetailsStep now enforces a minimum send amount per asset and blocks amounts above the connected wallet's actual balance, looked up via a new lib/wallet-balance.ts (Horizon fetch, fails open/never blocks when the balance can't be determined). - Field errors are now also validated on blur, not just on submit. Also fixes a pre-existing, unrelated bug that was blocking this repo's whole test suite from compiling: lib/create-bridgelet-client.ts had every template literal written with literally-escaped backticks (\` / \${) instead of real ones, a syntax error that made the file (and everything importing it, including the send flow) fail to parse. Fixed so `npm test` actually exercises the send flow's existing coverage, and added the missing @testing-library/dom devDependency the test suite needs. Tests: lib/validation/stellar-address.test.ts, lib/wallet-balance.test.ts, and new/updated coverage in details-step.test.tsx and connect-step.test.tsx (53 tests, all passing). Co-Authored-By: Claude Sonnet 5 --- .../send-form/steps/connect-step.test.tsx | 29 + .../send-form/steps/connect-step.tsx | 16 + .../send-form/steps/details-step.test.tsx | 150 + .../send-form/steps/details-step.tsx | 70 +- frontend/components/wallet-address-input.tsx | 10 +- frontend/lib/create-bridgelet-client.ts | 20 +- .../lib/validation/stellar-address.test.ts | 45 + frontend/lib/validation/stellar-address.ts | 25 + frontend/lib/wallet-balance.test.ts | 87 + frontend/lib/wallet-balance.ts | 74 + frontend/package-lock.json | 3629 +++++------------ frontend/package.json | 3 +- 12 files changed, 1538 insertions(+), 2620 deletions(-) create mode 100644 frontend/components/send-form/steps/details-step.test.tsx create mode 100644 frontend/lib/validation/stellar-address.test.ts create mode 100644 frontend/lib/validation/stellar-address.ts create mode 100644 frontend/lib/wallet-balance.test.ts create mode 100644 frontend/lib/wallet-balance.ts diff --git a/frontend/components/send-form/steps/connect-step.test.tsx b/frontend/components/send-form/steps/connect-step.test.tsx index 5e4f7fcf..5e8cd539 100644 --- a/frontend/components/send-form/steps/connect-step.test.tsx +++ b/frontend/components/send-form/steps/connect-step.test.tsx @@ -180,4 +180,33 @@ describe('ConnectStep', () => { expect(screen.getByRole('button', { name: /connect freighter wallet/i })).toBeEnabled(), ); }); + + // ── Destination address format validation (Issue #420) ────────────────── + + it('rejects a malformed address returned by Freighter instead of connecting', async () => { + mockConnectFreighter.mockResolvedValue({ publicKey: 'not-a-real-address' }); + const onConnected = vi.fn(); + const user = userEvent.setup(); + + render(); + await user.click(screen.getByRole('button', { name: /connect freighter wallet/i })); + + expect(await screen.findByRole('alert')).toHaveTextContent(/unexpected format/i); + expect(onConnected).not.toHaveBeenCalled(); + expect(mockPersistWallet).not.toHaveBeenCalled(); + }); + + it('discards a corrupted persisted wallet address instead of auto-connecting it', async () => { + mockLoadPersistedWallet.mockReturnValue({ publicKey: 'corrupted-value', type: 'freighter' }); + const onConnected = vi.fn(); + + render(); + + // Give the restore effect time to run. + await new Promise((r) => setTimeout(r, 50)); + + expect(onConnected).not.toHaveBeenCalled(); + expect(mockClearPersistedWallet).toHaveBeenCalledTimes(1); + expect(screen.getByRole('button', { name: /connect freighter wallet/i })).toBeInTheDocument(); + }); }); diff --git a/frontend/components/send-form/steps/connect-step.tsx b/frontend/components/send-form/steps/connect-step.tsx index 2603ce5b..5eb44d35 100644 --- a/frontend/components/send-form/steps/connect-step.tsx +++ b/frontend/components/send-form/steps/connect-step.tsx @@ -8,6 +8,7 @@ import { clearPersistedWallet, } from '@/lib/wallet'; import { ChainSelector } from '@/components/chain-selector'; +import { isValidStellarAddress } from '@/lib/validation/stellar-address'; type ConnectStepProps = { publicKey: string; @@ -64,6 +65,13 @@ export function ConnectStep({ publicKey, onConnected, extensionSupportedOverride if (publicKey) return; // parent already has a key — nothing to restore const saved = loadPersistedWallet(); if (saved?.publicKey) { + // Issue #420 — never trust a persisted address blindly; a corrupted + // or tampered localStorage value should never silently become the + // funding/recovery address for a payment. + if (!isValidStellarAddress(saved.publicKey)) { + clearPersistedWallet(); + return; + } onConnected(saved.publicKey); } // eslint-disable-next-line react-hooks/exhaustive-deps @@ -74,6 +82,14 @@ export function ConnectStep({ publicKey, onConnected, extensionSupportedOverride setError(null); try { const wallet = await connectFreighter(); + // Issue #420 — reject a malformed destination/funding address before + // it ever reaches the send form, rather than letting an invalid key + // silently flow through to submission. + if (!isValidStellarAddress(wallet.publicKey)) { + setStatus('error'); + setError('Freighter returned an address in an unexpected format. Please try reconnecting.'); + return; + } persistWallet(wallet); setStatus('idle'); onConnected(wallet.publicKey); diff --git a/frontend/components/send-form/steps/details-step.test.tsx b/frontend/components/send-form/steps/details-step.test.tsx new file mode 100644 index 00000000..0736728a --- /dev/null +++ b/frontend/components/send-form/steps/details-step.test.tsx @@ -0,0 +1,150 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { useState } from 'react'; +import { DetailsStep, validateDetails } from '@/components/send-form/steps/details-step'; +import type { SendFormState } from '@/components/send-form'; + +const getXlmUsdRate = vi.fn(); + +vi.mock('@/lib/xlm-price', async () => { + const actual = await vi.importActual('@/lib/xlm-price'); + return { + ...actual, + getXlmUsdRate: () => getXlmUsdRate(), + }; +}); + +const getAccountBalance = vi.fn(); + +vi.mock('@/lib/wallet-balance', () => ({ + getAccountBalance: (...args: unknown[]) => getAccountBalance(...args), +})); + +const VALID_PUBLIC_KEY = 'G' + 'A'.repeat(55); + +const INITIAL_STATE: SendFormState = { + publicKey: 'GABC', // not a valid Stellar address on purpose — see below + recipientName: '', + recipientEmail: '', + amountXlm: '', + assetCode: 'XLM', + memo: '', + expiresIn: 7 * 24 * 60 * 60, +}; + +function Harness({ + onNext = vi.fn(), + onBack = vi.fn(), + initialState = INITIAL_STATE, +}: { + onNext?: () => void; + onBack?: () => void; + initialState?: SendFormState; +}) { + const [state, setState] = useState(initialState); + return ( + setState((prev) => ({ ...prev, ...patch }))} + onBack={onBack} + onNext={onNext} + /> + ); +} + +describe('validateDetails — minimum amount (Issue #420)', () => { + it('rejects an amount below the minimum for the selected asset', () => { + expect( + validateDetails({ ...INITIAL_STATE, amountXlm: '0.5', assetCode: 'XLM' }).amountXlm, + ).toMatch(/minimum amount/i); + }); + + it('accepts an amount at or above the minimum', () => { + expect( + validateDetails({ ...INITIAL_STATE, amountXlm: '1', assetCode: 'XLM' }).amountXlm, + ).toBeUndefined(); + expect( + validateDetails({ ...INITIAL_STATE, amountXlm: '10', assetCode: 'XLM' }).amountXlm, + ).toBeUndefined(); + }); +}); + +describe('DetailsStep — sender balance guard (Issue #420)', () => { + beforeEach(() => { + vi.clearAllMocks(); + getXlmUsdRate.mockResolvedValue(0.5); + getAccountBalance.mockResolvedValue(null); + }); + + it('never queries the balance for a placeholder/invalid wallet key', async () => { + render(); + await waitFor(() => expect(getXlmUsdRate).toHaveBeenCalled()); + expect(getAccountBalance).not.toHaveBeenCalled(); + }); + + it('blocks submission and shows an inline error when the amount exceeds the wallet balance', async () => { + getAccountBalance.mockResolvedValue(5); + const onNext = vi.fn(); + const user = userEvent.setup(); + + render( + , + ); + + await user.type(screen.getByLabelText(/amount/i), '10'); + await waitFor(() => expect(getAccountBalance).toHaveBeenCalledWith(VALID_PUBLIC_KEY, 'XLM')); + + await user.click(screen.getByRole('button', { name: /review payment/i })); + + expect(onNext).not.toHaveBeenCalled(); + await waitFor(() => + expect(screen.getByText(/exceeds your wallet balance/i)).toBeInTheDocument(), + ); + }); + + it('allows submission when the amount is within the wallet balance', async () => { + getAccountBalance.mockResolvedValue(100); + const onNext = vi.fn(); + const user = userEvent.setup(); + + render( + , + ); + + await user.type(screen.getByLabelText(/amount/i), '10'); + await waitFor(() => expect(getAccountBalance).toHaveBeenCalled()); + + await user.click(screen.getByRole('button', { name: /review payment/i })); + expect(onNext).toHaveBeenCalledTimes(1); + }); +}); + +describe('DetailsStep — inline validation before submit (Issue #420)', () => { + beforeEach(() => { + vi.clearAllMocks(); + getXlmUsdRate.mockResolvedValue(0.5); + getAccountBalance.mockResolvedValue(null); + }); + + it('shows the amount error on blur, before any submit attempt', async () => { + const user = userEvent.setup(); + render(); + + const amountInput = screen.getByLabelText(/amount/i); + await user.click(amountInput); + await user.tab(); // blur without typing anything + + expect(screen.getByRole('alert')).toHaveTextContent(/enter an amount/i); + }); + + it('shows the malformed-email error on blur, before any submit attempt', async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByLabelText(/recipient email/i), 'not-an-email'); + await user.tab(); + + expect(screen.getByText(/valid email/i)).toBeInTheDocument(); + }); +}); diff --git a/frontend/components/send-form/steps/details-step.tsx b/frontend/components/send-form/steps/details-step.tsx index de710b0d..5a05cb57 100644 --- a/frontend/components/send-form/steps/details-step.tsx +++ b/frontend/components/send-form/steps/details-step.tsx @@ -4,11 +4,22 @@ import { useEffect, useState } from 'react'; import type { SendFormState } from '../index'; import { ChainSelector } from '../../chain-selector'; import { getXlmUsdRate, formatFiat } from '@/lib/xlm-price'; +import { isValidStellarAddress } from '@/lib/validation/stellar-address'; +import { getAccountBalance } from '@/lib/wallet-balance'; const SUPPORTED_ASSETS = ['XLM', 'USDC'] as const; const EMAIL_PATTERN = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; +/** + * Issue #420 — Minimum send amount per asset. For XLM this reflects + * Stellar's ~1 XLM base account reserve (the ephemeral account being + * funded must clear the network's minimum balance to exist at all); the + * same floor is applied to other supported assets for a simple, predictable + * rule rather than tracking a separate reserve model per asset. + */ +const MIN_AMOUNT: Record = { XLM: 1, USDC: 1 }; + type FieldErrors = { recipientEmail?: string; amountXlm?: string; @@ -27,6 +38,11 @@ export function validateDetails(state: SendFormState): FieldErrors { errors.amountXlm = 'Enter an amount.'; } else if (amount <= 0) { errors.amountXlm = 'Amount must be greater than 0.'; + } else { + const min = MIN_AMOUNT[state.assetCode] ?? MIN_AMOUNT['XLM']!; + if (amount < min) { + errors.amountXlm = `Minimum amount is ${min} ${state.assetCode || 'XLM'}.`; + } } if (!SUPPORTED_ASSETS.includes(state.assetCode as (typeof SUPPORTED_ASSETS)[number])) { @@ -49,6 +65,12 @@ export function DetailsStep({ state, onChange, onBack, onNext }: DetailsStepProp const [touched, setTouched] = useState(false); const [usdRate, setUsdRate] = useState(null); + // Issue #420 — sender balance, used to block amounts above what the + // connected wallet actually holds. Only looked up once we have a + // well-formed Stellar address (the funding source from ConnectStep) — + // never fired for a placeholder/invalid key. + const [balance, setBalance] = useState(null); + useEffect(() => { let cancelled = false; getXlmUsdRate().then((rate) => { @@ -59,19 +81,43 @@ export function DetailsStep({ state, onChange, onBack, onNext }: DetailsStepProp }; }, []); + useEffect(() => { + let cancelled = false; + if (!isValidStellarAddress(state.publicKey)) { + setBalance(null); + return; + } + getAccountBalance(state.publicKey, state.assetCode).then((b) => { + if (!cancelled) setBalance(b); + }); + return () => { + cancelled = true; + }; + }, [state.publicKey, state.assetCode]); + useEffect(() => { if (touched) setErrors(validateDetails(state)); }, [state.recipientEmail, state.amountXlm, state.assetCode, touched]); + const amount = Number(state.amountXlm); + const insufficientBalance = + balance !== null && !Number.isNaN(amount) && amount > 0 && amount > balance; + + function markTouched() { + if (!touched) { + setTouched(true); + setErrors(validateDetails(state)); + } + } + function handleSubmit(e: React.FormEvent) { e.preventDefault(); const nextErrors = validateDetails(state); setErrors(nextErrors); setTouched(true); - if (Object.keys(nextErrors).length === 0) onNext(); + if (Object.keys(nextErrors).length === 0 && !insufficientBalance) onNext(); } - const amount = Number(state.amountXlm); const showConversion = state.assetCode === 'XLM' && usdRate !== null && !Number.isNaN(amount) && amount > 0; @@ -106,6 +152,7 @@ export function DetailsStep({ state, onChange, onBack, onNext }: DetailsStepProp type="email" value={state.recipientEmail} onChange={(e) => onChange({ recipientEmail: e.target.value })} + onBlur={markTouched} placeholder="recipient@example.com" aria-invalid={errors.recipientEmail ? true : undefined} aria-describedby={errors.recipientEmail ? 'recipient-email-error' : undefined} @@ -135,11 +182,14 @@ export function DetailsStep({ state, onChange, onBack, onNext }: DetailsStepProp step="any" value={state.amountXlm} onChange={(e) => onChange({ amountXlm: e.target.value })} + onBlur={markTouched} placeholder="0.00" - aria-invalid={errors.amountXlm ? true : undefined} - aria-describedby={errors.amountXlm ? 'amount-error' : undefined} + aria-invalid={errors.amountXlm || insufficientBalance ? true : undefined} + aria-describedby={ + errors.amountXlm ? 'amount-error' : insufficientBalance ? 'amount-balance-error' : undefined + } className={`block w-full rounded-lg border px-3 py-2 text-sm text-slate-900 placeholder-slate-400 focus:outline-none focus:ring-1 dark:bg-slate-800 dark:text-slate-100 dark:placeholder-slate-500 ${ - errors.amountXlm + errors.amountXlm || insufficientBalance ? 'border-red-400 focus:border-red-500 focus:ring-red-500 dark:border-red-600' : 'border-slate-300 focus:border-slate-500 focus:ring-slate-500 dark:border-slate-600 dark:focus:border-slate-400 dark:focus:ring-slate-400' }`} @@ -165,6 +215,16 @@ export function DetailsStep({ state, onChange, onBack, onNext }: DetailsStepProp {errors.amountXlm}

)} + {/* Issue #420 — amount above sender balance blocked client-side */} + {!errors.amountXlm && insufficientBalance && ( + + )} {errors.assetCode && (
+ +
+

+ {submittingLabel()} +

+

+ {submitPhase === 'confirming' + ? "Your transaction has been submitted and is waiting for network confirmation. Don't close this page." + : 'Please wait — this only takes a few seconds.'} +

+
+
+ )} + + {/* Issue #421 — timeout handling: a non-blocking notice if confirmation is unusually slow */} + {submitting && showSlowWarning && ( +
+ This is taking longer than usual. The Stellar network may be busy — your payment has not + failed, and we'll keep waiting for a confirmation. +
+ )} +