Skip to content

Parse commitment amounts as exact fixed-point values - #2121

Merged
1nonlypiece merged 2 commits into
Commitlabs-Org:masterfrom
Abioladory123:fix/exact-amount-parsing
Oct 24, 2026
Merged

1nonlypiece merged 2 commits into
Commitlabs-Org:masterfrom
Abioladory123:fix/exact-amount-parsing

Conversation

@Abioladory123

Copy link
Copy Markdown
Contributor

Description

The wizard parsed the amount with Number(), so 1e3, 0x10 and " 5 " were accepted and amounts were stored and summed as floats. This adds a strict decimal parser that produces exact base-unit bigints (7 decimals) and uses it for validation, display, storage and the penalty/yield math.

Closes #2081

What changed

  • src/lib/amount.ts (new)
    • parseAmount(input, decimals = 7) accepts only ^\d+(\.\d+)?$ after trimming. Rejections carry a code (empty, invalid_characters, too_many_decimals, zero, above_max) with a user-facing message map.
    • parseDays(input, { min, max }) is digits only (1e1, 14.0, -3 rejected) and range-checked (empty, invalid_characters, below_min, above_max).
    • formatAmount(value, decimals, { maxFraction }) renders bigints with grouping.
    • penaltyOf and yieldOf do BigInt basis-point math.
  • src/lib/commitments.ts
    • Commitment.amount is now a base-unit decimal string.
    • normalizeStoredAmount is the single conversion used on load.
    • Schema version 2 to 3, so legacy payloads are rewritten in place on first load.
  • src/app/create/page.tsx
    • Uses the parsers, with inline reasons and pattern/inputMode on the inputs.
    • The review step shows full precision.
  • src/app/dashboard/page.tsx
    • Totals, exit breakdown and payout use bigint.
    • Card headings and metrics are abbreviated; payout, settled yield, penalty paid and the exit dialog are full precision.

Decisions

  • Whitespace: leading and trailing whitespace is trimmed (" 5 " is 5); whitespace inside the number ("1 000") is rejected. Documented in the parseAmount comment and covered in tests.
  • Forms: .5 and 5. are rejected, as the issue's regex implies. Leading zeros are allowed (007 is 7).
  • Max: MAX_AMOUNT = 1_000_000_000_000n base units (100,000 XLM), a named constant mirroring MAX_AMOUNT in contracts/escrow/src/lib.rs. above_max is checked after zero.
  • Rounding: penalty and yield round down to a whole stroop, once at the end. This matches compute_refund_amount and calculate_accrued_yield in the escrow contract, so the UI never shows more than the contract would pay. Refund is amount - penalty, so no stroop is lost. Example: 33 stroops at 5% is 1.65, shown as penalty 1 and refund 32.
  • Summaries: maxFraction truncates rather than rounds. A non-zero value that would truncate to zero shows as <0.01, so 1 stroop no longer displays as 0.
  • Legacy records: numbers are converted with toFixed(7) and then scaled, which removes float noise (0.1 + 0.2 becomes 0.3). Negative, NaN, exponent-form and non-digit values are dropped through the existing dropped counter and the dashboard notice; nothing throws. Legacy values above the max are kept, not dropped, so existing records stay visible.
  • Percent to bps: Math.round(percent * 100), valid for the 2-decimal percentages in TYPES (4.5 becomes 450).

Not changed

  • fmt(Number(...)) stays for the wallet XLM balance and for on-chain event amounts. The balance is a Horizon display string. The event amount is raw stroops that the dashboard shows as XLM, which looks like a separate unit bug, so I left it for its own issue.
  • The UI APYs in TYPES (4.5/9/16) differ from the contract's annual_yield_bps (500/700/1000). Pre-existing and out of scope.
  • No contract, package.json, lockfile, workflow or script changes.

One thing to be aware of: records are now written as schema v3 with string amounts. An older build reading that payload would drop them, so rolling back the deploy after users have saved would discard those local demo records.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Testing

npm run typecheck and npm run circular are clean. npm test:

Test Files  1 failed | 15 passed (16)
     Tests  1 failed | 194 passed (195)

The one failure is src/lib/soroban/errors.test.ts > matches the Rust contract error variants and numeric codes (expects 15 contract errors, finds 17). It is pre-existing: I ran it on a clean checkout of upstream/master (0f6f056) before making any changes and it failed the same way (1 failed, 89 passed, 90 total).

New tests:

  • src/lib/__tests__/amount.test.ts (52): every rejected input with its reason, 1-stroop accept and 8-decimal reject, exactly max accepted and max + 1 stroop rejected, parseDays cases, format round trips and truncation, hand-calculated penalty and yield including non-whole-stroop cases.
  • src/lib/__tests__/commitments.test.ts: legacy v1 and v2 numeric records converted and rewritten, malformed stored amounts dropped without throwing, and a string round trip.
  • src/app/create/create.amount.test.tsx: the wizard shows the inline reason and blocks Review for bad input, trims valid input, shows exact review values, and stores '1' for 1 stroop.
  • dashboard.a11y.test.tsx: seed amounts changed to base-unit strings to match the new type. Assertions are unchanged.

The checklist's test:coverage / lint scripts don't exist in package.json, so I could not run coverage or lint.

Checklist

  • My code follows the code style and standards of this project.
  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added unit/integration tests that prove my fix is effective.
  • Coverage of new logic at least 95% (no coverage script available, not measured).
  • Linter (no lint script available).
  • Screenshots for UI changes (not attached).

Add parseAmount/parseDays that reject malformed input with specific reasons, keep amounts as bigint base units (7 decimals, max 1e12 like the contract), compute penalty and yield with basis point math, and convert legacy numeric records on load. The wizard and dashboard use them.
@vercel

vercel Bot commented Oct 23, 2026

Copy link
Copy Markdown

@Abioladory123 is attempting to deploy a commit to the 1nonly's projects Team on Vercel.

A member of the Team first needs to authorize it.

@Abioladory123

Copy link
Copy Markdown
Contributor Author

@1nonlypiece please review pr and merge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@1nonlypiece
1nonlypiece merged commit 74b7dd9 into Commitlabs-Org:master Oct 24, 2026
1 check failed
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.

[frontend] Parse commitment amounts as exact fixed-point values and reject malformed numeric input

2 participants