-
Notifications
You must be signed in to change notification settings - Fork 107
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
feat(spaceward): fix max value in staking #882
Conversation
WalkthroughWalkthroughThe changes in the pull request focus on the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Hey @alex-nax and thank you for opening this pull request! 👋🏼 It looks like you forgot to add a changelog entry for your changes. Make sure to add a changelog entry in the 'CHANGELOG.md' file. |
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.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
spaceward/src/features/staking/StakeModal.tsx (1)
26-26
: Approved: Good fix formaxAmount
calculationThe change effectively addresses the PR objective by ensuring
maxAmount
is non-negative and accounts for potential fees. This is a good improvement.Consider updating the comment to be more specific:
- const maxAmount = Math.max(ward - 1e-14 /* should be enough to pay fee */, 0); + const maxAmount = Math.max(ward - 1e-14 /* Buffer for transaction fees */, 0);This makes the purpose of the buffer clearer to other developers.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- spaceward/src/features/staking/StakeModal.tsx (3 hunks)
Additional comments not posted (2)
spaceward/src/features/staking/StakeModal.tsx (2)
165-165
: Approved: Good addition of default valueAdding a default value of 0 for the commission rate calculation is a good practice. It prevents potential NaN results if the rate is undefined, improving the robustness of the code.
190-191
: Verify APR calculation decimalsThe use of BigInt for the APR calculation improves precision, which is good. However, the comment suggests there might be an issue with the decimals.
Please verify the correct number of decimals for the APR calculation. If the issue persists, consider using a decimal library like
decimal.js
for more accurate financial calculations.To help investigate this, you can run the following script to find all occurrences of APR calculations in the codebase:
This will help identify if there are consistent patterns or discrepancies in APR calculations across the project.
Summary by CodeRabbit
New Features
Bug Fixes