Skip to content

Conversation

aaroncox
Copy link
Member

No description provided.

@aaroncox aaroncox requested a review from Copilot July 15, 2025 20:28
Copilot

This comment was marked as outdated.

aaroncox and others added 4 commits July 15, 2025 13:33
@aaroncox aaroncox requested a review from Copilot July 15, 2025 20:51
@wharfkit wharfkit deleted a comment from Copilot AI Jul 15, 2025
@wharfkit wharfkit deleted a comment from Copilot AI Jul 15, 2025
@wharfkit wharfkit deleted a comment from Copilot AI Jul 15, 2025
Copy link

@Copilot Copilot AI left a 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 () {
Copy link
Preview

Copilot AI Jul 15, 2025

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.

Suggested change
test('frac_by_us now', async function () {
test('frac_by_us_comparison', async function () {

Copilot uses AI. Check for mistakes.

Comment on lines +74 to +75
const multiplier = intToBigDecimal(Math.pow(10, 15))
return UInt128.from(base.multiply(weight).divide(multiplier, 15).ceil().getValue())
Copy link
Preview

Copilot AI Jul 15, 2025

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.

Suggested change
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.

Comment on lines +61 to +65
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)
Copy link
Preview

Copilot AI Jul 15, 2025

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.

Suggested change
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
Copy link
Preview

Copilot AI Jul 15, 2025

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.

Suggested change
const precision = 15
const precision = PRECISION;

Copilot uses AI. Check for mistakes.

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)
Copy link
Preview

Copilot AI Jul 15, 2025

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.

Suggested change
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)
Copy link
Preview

Copilot AI Jul 15, 2025

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.

Suggested change
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.

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.

1 participant