Skip to content

Finish the price-math extraction: Checkout.tsx still hand-rolls 7 inline copies of usdcToFiat (FIN 51) #52

Description

@gitchadd
SEC UX AUTO YAGNI DELTA FIN
0 5 90 90 70 51

Found by: adversarial review of #50 (architecture / YAGNI lens). Not a correctness bug — the inline copies are byte-identical floor-division, so they produce the same result — a maintainability + doc-accuracy issue and a good fix run candidate.

Symptom

The price-math.ts docstring claims usdcToFiat "mirrors the breakdown math the UI already does inline so the two stay in lockstep":

* fiat-denominated checkout mode (`fiatChargeAmount`) is the only consumer of
* the inversion; the forward `usdcToFiat` mirrors the breakdown math the UI
* already does inline so the two stay in lockstep.

Nothing enforces lockstep. Checkout.tsx does not import price-math at all and still carries 7 inline (x * buyPrice) / 1_000_000n copies — lines 117, 145, 160, 164, 165, 224, 237:

const chargedFiat = (chargedUsdc * state.buyPrice) / 1_000_000n;
// Fee follows the DELTA (charged amount), not the gross order:
// - credit covers fully (chargedUsdc == 0): no Diamond order, no fee
// - chargedUsdc > 0 + chargedUsdc ≤ smallOrderThreshold: fee applies
// - chargedUsdc > smallOrderThreshold: waived
// Mirrors the Diamond's on-chain fee logic, which evaluates the
// small-order threshold against `order.amount` (= delta when credit
// is netted on-chain by the integrator).
const feeUsdc =
state.smallOrderThreshold !== null &&
state.smallOrderFixedFee !== null &&
chargedUsdc > 0n &&
chargedUsdc <= state.smallOrderThreshold
? state.smallOrderFixedFee
: 0n;
const feeFiat = (feeUsdc * state.buyPrice) / 1_000_000n;
const totalFiat = chargedFiat + feeFiat;
// Subtotal-without-credit: shown when credit > 0 to make the deduction
// visible. Equals what the user would have paid pre-credit.
const grossFiat = (orderUsdc * state.buyPrice) / 1_000_000n;
const creditFiat = (credit * state.buyPrice) / 1_000_000n;

#50 added a fresh copy at L117 (feeFiatLabel) and edited L164 (grossFiat) rather than switching to the helper it was extracting in the same commit. order-machine.ts did adopt the helper (the good half); the view did not — so there are now two sources of truth for one formula that agree only by coincidence.

Fix (mechanical, behavior-preserving)

  • Import usdcToFiat into Checkout.tsx; replace the 7 inline copies with usdcToFiat(a, state.buyPrice).
  • Collapse the two near-identical gross-fiat IIFEs in order-machine.ts (routingFiat ~L728-737, contextFiat ~L768-785) into one shared price-math helper: grossFiat = usdcToFiat(order) + usdcToFiat(fee).

Net effect: every "amount → fiat" number in the buy path derives from price-math, making the docstring's lockstep claim structurally true. Deletes duplication, adds zero surface.

Doc nit (bundle)

README says the "You pay" line "lands on exactly the number you passed":

widgets/README.md

Lines 1266 to 1270 in 8c08cd8

the protocol's small-order fee is **baked in**, not added on top. The
widget reads `getPriceConfig(currency).buyPrice`, backs the fee out of the
total, and derives the USDC order amount so the **"You pay"** line lands on
exactly the number you passed. (Compare `usdcAmount`, where the fee is
charged *on top* of the USDC you specify.)

It lands to the nearest micro-USDC (round-half-up + floor re-display leaves up to a ~½-micro-USDC-of-price residual; the round-trip test asserts diff <= BUY_PRICE, i.e. explicitly not exact). Soften "exactly" to "to the nearest micro-USDC".

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentationenhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions