fix(scripts): scale token amounts without a floating-point intermediary#194
fix(scripts): scale token amounts without a floating-point intermediary#194Nexory wants to merge 1 commit into
Conversation
The bridge and mint CLI handlers scaled the user-entered amount with `BigInt(Math.floor(amount * 10 ** decimals))`, where `amount` is a `parseFloat`-ed number. Evaluating `amount * 10 ** decimals` in IEEE-754 double precision loses accuracy: `1.005 * 1e9` is `1004999999.9999999`, so `Math.floor` produces `1_004_999_999` lamports instead of the intended `1_005_000_000`. `1.000001` at 6 decimals drops the trailing unit (`1_000_000` instead of `1_000_001`), and 18-decimal (wei) amounts exceed `Number.MAX_SAFE_INTEGER` and lose arbitrary trailing precision. Add a small `parseTokenAmount(value, decimals)` helper backed by viem's `parseUnits`, which parses the decimal string directly with no float intermediary, and use it in all six call sites: - bridge-sol, bridge-sol-with-bc, bridge-spl, bridge-wrapped-token (9 or mint-decimals) - bridge-call (18, ETH -> wei) - spl/mint (mint decimals) The amount/value Zod fields now keep the validated string instead of transforming to a float, so the raw decimal input reaches parseUnits intact. Logging is unchanged (the field is still a string). Verified locally: - bun test src/internal/amount.test.ts : 3 pass (documents the legacy float loss and pins the corrected output) - tsc --noEmit : 0 errors across the scripts package
🟡 Heimdall Review Status
|
|
Review Error for mohamedelhalosy2023-stack @ 2026-06-22 00:23:27 UTC |
|
Review Error for mohamedelhalosy2023-stack @ 2026-06-22 10:04:01 UTC |
|
Review Error for mohamedelhalosy2023-stack @ 2026-06-22 19:10:37 UTC |
|
Good catch on the Math.floor(amount * 10**decimals) precision loss. One thing worth double-checking before this is called fixed: the Zod schema upstream still does .transform((val) => parseFloat(val)) on args.amount. If that transform runs before the value reaches parseTokenAmount, the amount has already been round-tripped through an IEEE-754 double before parseUnits ever sees it — so for high-precision 18-decimal inputs specified with many significant digits, precision could be lost at the parsing stage even after the scaling math itself is fixed. |
|
Review Error for mohamedelhalosy2023-stack @ 2026-07-04 11:02:43 UTC |
|
Thanks for taking a look @FernLovesCrypto ! I think this is reading the pre-change schema. The PR actually removes the The high-precision case is already covered in |
What
The Solana bridge and SPL mint CLI handlers scale the user-entered amount with a floating-point intermediary:
args.amountis aparseFloat-ed number (from the Zod.transform((val) => parseFloat(val))), andamount * 10 ** decimalsis evaluated in IEEE-754 double precision. That loses accuracy for ordinary decimal inputs.Why it is wrong
1.005 * 1e9evaluates to1004999999.9999999, soMath.floorrounds it down. The on-chain amount ends up smaller than what the user typed. The magnitude is small for 9/6-decimal tokens (one smallest-unit), but for 18-decimal (wei) values any amount past ~15-16 significant digits loses precision because the product exceedsNumber.MAX_SAFE_INTEGER.Fix
Add a small
parseTokenAmount(value, decimals)helper backed by viem'sparseUnits(already a dependency), which parses the decimal string directly with no float intermediary, and use it at all six call sites:bridge-sol,bridge-sol-with-bc,bridge-spl,bridge-wrapped-tokenbridge-call(ETH -> wei)spl/mintThe
amount/valueZod fields now keep the validated string instead of transforming to a float, so the raw decimal input reachesparseUnitsintact. Logging is unchanged (the field is still a string).parseUnitstruncates over-precision inputs the same way the previousMath.floordid, so no input that worked before changes behavior.Verification (local)
The test documents the legacy float loss (
1.005 @ 9 -> 1_004_999_999) and pins the corrected output (-> 1_005_000_000).Notes
scriptspackage (the workflows cover the Forge and Solana programs), so the verification above was run locally. Happy to wire abun test/tscstep into CI forscriptsin a follow-up if useful.