-
-
Notifications
You must be signed in to change notification settings - Fork 7
Rewrite math using bigDecimal and Antelope Integers #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR rewrites the mathematical calculations in the resources library to use bigDecimal for high precision arithmetic and integrates Antelope Integers for better type safety. The changes modernize the math operations while maintaining compatibility with existing functionality through legacy method variants.
- Replaces JavaScript number operations with bigDecimal for precision-critical calculations
- Integrates Antelope Integer types (UInt128, Int64) throughout the codebase
- Adds comprehensive migration tests to ensure legacy and modern implementations produce identical results
Reviewed Changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
test/migrate.ts | New comprehensive test suite comparing legacy vs modern implementations |
src/powerup/abstract.ts | Core resource calculation methods rewritten with bigDecimal and legacy variants |
src/powerup/cpu.ts | CPU resource calculations updated with modern types and precision handling |
src/powerup/net.ts | Network resource calculations updated with modern types and precision handling |
src/index.ts | Added bigDecimal utility function and updated imports |
package.json | Updated dependencies for Antelope v1.1.0 and mock-data v1.3.0 |
test/data/*.json | Updated test data files with consistent headers structure |
Comments suppressed due to low confidence (1)
test/migrate.ts:11
- [nitpick] The variable name 'us' is ambiguous. It should be renamed to 'microseconds' or 'usValue' to clearly indicate it represents microseconds.
let us: number
test/migrate.ts
Outdated
`modern=${String(modern)} legacy=${legacy} us=${us}` | ||
) | ||
}) | ||
test('frac_by_us now', async function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The test name 'frac_by_us now' contains an unclear word 'now'. It should be 'frac_by_us' or clarify what 'now' refers to.
test('frac_by_us now', async function () { | |
test('frac_by_us_comparison', async function () { |
Copilot uses AI. Check for mistakes.
const multiplier = intToBigDecimal(Math.pow(10, 15)) | ||
return UInt128.from(base.multiply(weight).divide(multiplier, 15).ceil().getValue()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic number '15' should be extracted to a named constant to improve code readability and maintainability.
const multiplier = intToBigDecimal(Math.pow(10, 15)) | |
return UInt128.from(base.multiply(weight).divide(multiplier, 15).ceil().getValue()) | |
const multiplier = intToBigDecimal(Math.pow(10, DIVISION_PRECISION)) | |
return UInt128.from(base.multiply(weight).divide(multiplier, DIVISION_PRECISION).ceil().getValue()) |
Copilot uses AI. Check for mistakes.
const precision = 15 | ||
const converted = intToBigDecimal(this.us_to_weight(usage.cpu, us)) | ||
const current = intToBigDecimal(this.weight) | ||
const multiplier = intToBigDecimal(Math.pow(10, precision)) | ||
const frac = converted.divide(current, precision).multiply(multiplier) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The precision value '15' is duplicated across multiple methods. Consider extracting it to a shared constant to ensure consistency and easier maintenance.
const precision = 15 | |
const converted = intToBigDecimal(this.us_to_weight(usage.cpu, us)) | |
const current = intToBigDecimal(this.weight) | |
const multiplier = intToBigDecimal(Math.pow(10, precision)) | |
const frac = converted.divide(current, precision).multiply(multiplier) | |
const converted = intToBigDecimal(this.us_to_weight(usage.cpu, us)) | |
const current = intToBigDecimal(this.weight) | |
const multiplier = intToBigDecimal(Math.pow(10, PRECISION)) | |
const frac = converted.divide(current, PRECISION).multiply(multiplier) |
Copilot uses AI. Check for mistakes.
return Math.floor(frac * Math.pow(10, 15)) | ||
} | ||
frac_by_bytes(usage: SampleUsage, bytes: Int64Type): Int64 { | ||
const precision = 15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The precision value '15' is duplicated across multiple methods. Consider extracting it to a shared constant to ensure consistency and easier maintenance.
const precision = 15 | |
const precision = PRECISION; |
Copilot uses AI. Check for mistakes.
src/powerup/abstract.ts
Outdated
if (new_exponent <= 0.0) { | ||
return max_price | ||
} else { | ||
const util_weight = new BN(utilization) / weight | ||
price += (max_price - min_price) * Math.pow(util_weight, new_exponent) | ||
const util_weight = intToBigDecimal(utilization).divide(intToBigDecimal(weight), 18) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The precision value '18' should be extracted to a named constant to improve code readability and maintainability.
const util_weight = intToBigDecimal(utilization).divide(intToBigDecimal(weight), 18) | |
const util_weight = intToBigDecimal(utilization).divide(intToBigDecimal(weight), DECIMAL_PRECISION) |
Copilot uses AI. Check for mistakes.
return this.max_price.value | ||
} else { | ||
// const util_weight = utilization.dividing(weight) | ||
const util_weight = intToBigDecimal(utilization).divide(intToBigDecimal(weight), 18) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The precision value '18' is duplicated. Consider extracting it to a shared constant to ensure consistency and easier maintenance.
const util_weight = intToBigDecimal(utilization).divide(intToBigDecimal(weight), 18) | |
const util_weight = intToBigDecimal(utilization).divide(intToBigDecimal(weight), DECIMAL_PRECISION) |
Copilot uses AI. Check for mistakes.
No description provided.