Skip to content
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

Kamino sdk small fixes #388

Merged
merged 1 commit into from
Oct 16, 2023
Merged

Kamino sdk small fixes #388

merged 1 commit into from
Oct 16, 2023

Conversation

alittlezz
Copy link
Contributor

@alittlezz alittlezz commented Oct 16, 2023

Summary by CodeRabbit

  • Bug Fix: Improved data reading accuracy in the Kamino SDK by updating the method used to read unsigned 64-bit integers. This change enhances the reliability of the driftRebalance, periodicRebalance, and utils functions.
  • Refactor: Updated the pricePercentageRebalance and pricePercentageWithResetRebalance functions to assign certain variables as strings directly, removing the usage of the Decimal class. The impact on user experience is currently unclear, but this change simplifies the codebase.

@coderabbitai
Copy link

coderabbitai bot commented Oct 16, 2023

Walkthrough

The changes primarily focus on updating the method used to read unsigned 64-bit integers from the buffer across various files. The readBigUint64LE method is replaced with readBigUInt64LE for correct value reading. Additionally, some changes involve removing the usage of the Decimal class and assigning string values directly.

Changes

File Path Summary
.../driftRebalance.ts Updated method for reading unsigned 64-bit integers from the buffer.
.../periodicRebalance.ts Replaced readBigUint64LE with readBigUInt64LE for correct value reading.
.../pricePercentageRebalance.ts Removed usage of Decimal class and assigned string values directly.
.../pricePercentageWithResetRebalance.ts Removed conversion to Decimal objects and assigned strings directly.
.../utils/utils.ts Updated readBigUint128LE function to use the correct method name.

"In the land of code, where the shadows lie,

A rabbit hopped, with a twinkle in its eye. 🐇✨

With each leap, a bug was squashed,

And the code's performance, it was not lost. 🚀💻

Now the integers read just right,

And the strings shine in the moonlight. 🌙📜

Oh, what a joy, this code to celebrate,

In the land of GitHub, where the coders create!" 🎉🎈


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
  • Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
    resume reviews, enter @coderabbitai resume as any issue or review comment.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 1

Commits Files that changed from the base of the PR and between 1012375 and 8b4bfcf.
Files selected for processing (5)
  • packages/kamino-sdk/src/rebalance_methods/driftRebalance.ts (2 hunks)
  • packages/kamino-sdk/src/rebalance_methods/periodicRebalance.ts (2 hunks)
  • packages/kamino-sdk/src/rebalance_methods/pricePercentageRebalance.ts (1 hunks)
  • packages/kamino-sdk/src/rebalance_methods/pricePercentageWithResetRebalance.ts (1 hunks)
  • packages/kamino-sdk/src/utils/utils.ts (1 hunks)
Files skipped from review due to trivial changes (2)
  • packages/kamino-sdk/src/rebalance_methods/periodicRebalance.ts
  • packages/kamino-sdk/src/utils/utils.ts
Additional comments (Suppressed): 4
packages/kamino-sdk/src/rebalance_methods/pricePercentageRebalance.ts (1)
  • 132-133: The variables lowerSqrtPriceX64 and upperSqrtPriceX64 are no longer being converted to Decimal objects but are directly assigned as strings. This change could potentially affect the subsequent calculations where these variables are used. Ensure that the change in data type does not introduce bugs or inaccuracies in the calculations.
packages/kamino-sdk/src/rebalance_methods/driftRebalance.ts (2)
  • 161-165: The change from readBigUint64LE to readBigUInt64LE is correct as it ensures the correct reading of the secondsPerTick and direction parameters. This change should not introduce any breaking changes as the method signatures remain the same and the returned values are still converted to Decimal objects.

  • 172-176: The change from readBigUint64LE to readBigUInt64LE is correct as it ensures the correct reading of the lastDriftTimestamp parameter. This change should not introduce any breaking changes as the method signature remains the same and the returned value is still converted to a Decimal object.

packages/kamino-sdk/src/rebalance_methods/pricePercentageWithResetRebalance.ts (1)
  • 215-216: The readBigUint128LE function is now directly converting the buffer to a string, instead of first converting it to a Decimal object. Ensure that this change does not affect any calculations or operations that rely on lowerResetSqrtPriceX64 and upperResetSqrtPriceX64 being Decimal objects.

Comment on lines 212 to 213
let resetLowerRangeBps = new Decimal(params.find((param) => param.label == 'resetLowerRangeBps')?.value.toString()!);
let resetUpperRangeBps = new Decimal(params.find((param) => param.label == 'resetUpperRangeBps')?.value!.toString()!);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of the non-null assertion operator (!) after value assumes that the value property will always exist in the found parameter object. If a parameter with the specified label is not found, or if the found parameter object does not have a value property, this will result in a runtime error. Consider adding error handling or a default value to prevent potential issues.

@alittlezz alittlezz merged commit 1e11aec into master Oct 16, 2023
3 checks passed
@alittlezz alittlezz deleted the vali/small-fixes-sdk branch October 16, 2023 15:41
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.

2 participants