refactor: extract shared Decimal→BigInt conversion helper - #167
Merged
dev-fani merged 1 commit intoAug 30, 2026
Merged
Conversation
Both the escrow repository and the analytics reader converted Prisma Decimal amounts to BigInt via .toFixed() and carried the same warning: Decimal.toString() switches to exponential notation past 21 significant digits, so BigInt() can't parse an i128::MAX-sized amount (39 digits) rendered as "1.7...e+38". The analytics copy even pointed back at the escrow one, so any future module touching an amount would have to rediscover the pitfall. Move the conversion into a single decimalToBigInt helper in shared/database with the explanation as its doc comment, and switch both call sites to it. Add unit tests covering the exponential-notation case. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
|
@lekescrew22 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #107
Closes #108
Closes #109
Closes #110
Summary
Two modules converted a Prisma
Decimalamount to aBigIntvia.toFixed()— and both carried the same multi-line comment explaining why that's necessary:Decimal.toString()switches to exponential notation past 21 significant digits, soBigInt()can't parse ani128::MAX-sized amount (39 digits) rendered as"1.7...e+38". The analytics reader's copy even pointed back at the escrow repository's, meaning the next module to touch an amount would have to rediscover the pitfall — or trip over it.This PR extracts that knowledge into a single shared helper so it lives in exactly one place.
Changes
src/shared/database/decimal.ts(new) —decimalToBigInt(value: Prisma.Decimal): bigint, with the exponential-notation explanation as its doc comment. Placed inshared/databasealongside the other Prisma concerns.src/shared/database/decimal.spec.ts(new) — unit tests: small values, zero, negatives, and the 39-digiti128::MAXcase that.toString()renders exponentially.src/shared/database/index.ts— exports the new helper.src/modules/escrow/infrastructure/prisma-escrow-repository.ts—toDomainusesdecimalToBigInt; duplicated comment removed.src/modules/analytics/infrastructure/prisma-analytics-reader.ts—getGmvByTokenusesdecimalToBigInt; the comment pointing at the escrow file removed. A null_sum.amount(empty group) still maps to0n, matching the previous?? '0'behavior.Why
.toFixed()and not.toString()?Verified against Prisma's
Decimal(decimal.js) with the reali128::MAXvalue170141183460469231731687303715884105727:.toString()1.70141183460469231731687303715884105727e+38→BigInt()throwsSyntaxError.toFixed()170141183460469231731687303715884105727→ parses cleanlyTesting
decimal.spec.ts: 4/4 pass.tsc --noEmit: no errors in any file touched by this PR.