-
Notifications
You must be signed in to change notification settings - Fork 49
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
fix: decimals in transfer #2589
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces modifications to three components related to resource management and transfer in the client-side application. The changes primarily focus on improving resource display precision, enhancing error handling in resource transfers, and updating component logic for fetching and displaying entity resources. The modifications include adjusting decimal place formatting, implementing asynchronous transfer handling, and refining component value retrieval methods. Changes
Sequence DiagramsequenceDiagram
participant User
participant RealmTransfer
participant ResourceService
User->>RealmTransfer: Initiate Resource Transfer
RealmTransfer->>RealmTransfer: Validate Resources
RealmTransfer->>ResourceService: send_resources_multiple(resources)
alt Transfer Successful
ResourceService-->>RealmTransfer: Transfer Completed
RealmTransfer-->>User: Show Success Confirmation
else Transfer Failed
ResourceService-->>RealmTransfer: Error
RealmTransfer->>RealmTransfer: Log Error
RealmTransfer-->>User: Show Error Message
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
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
CodeRabbit Configuration 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.
The changes look good overall, adding decimal support and improving error handling in the transfer function. The consistent use of 2 decimal places across resource displays and proper type handling for decimal conversion are good improvements. The error handling with try/catch is a good addition, though it could be enhanced with user-visible error feedback. The code organization is clean with good separation between display and transfer logic.
Thanks for using MentatBot. Give comments a 👍 or 👎 to help me improve!
const handleTransfer = useCallback(async () => { | ||
setIsLoading(true); | ||
const cleanedCalls = calls.map(({ sender_entity_id, recipient_entity_id, resources }) => ({ | ||
sender_entity_id, | ||
recipient_entity_id, | ||
resources: [resources[0], BigInt(resources[1]) * BigInt(1000)], | ||
resources: [resources[0], BigInt(Number(resources[1]) * 1000)], |
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.
Consider using a constant for the multiplier (1000) to make the code more maintainable and document its purpose. For example: const PRECISION_MULTIPLIER = 1000;
const handleTransfer = useCallback(async () => { | |
setIsLoading(true); | |
const cleanedCalls = calls.map(({ sender_entity_id, recipient_entity_id, resources }) => ({ | |
sender_entity_id, | |
recipient_entity_id, | |
resources: [resources[0], BigInt(resources[1]) * BigInt(1000)], | |
resources: [resources[0], BigInt(Number(resources[1]) * 1000)], | |
const PRECISION_MULTIPLIER = 1000; // Multiplier used to handle decimal precision in resource transfers | |
const handleTransfer = useCallback(async () => { | |
setIsLoading(true); | |
const cleanedCalls = calls.map(({ sender_entity_id, recipient_entity_id, resources }) => ({ | |
sender_entity_id, | |
recipient_entity_id, | |
resources: [resources[0], BigInt(Number(resources[1]) * PRECISION_MULTIPLIER)], |
try { | ||
await send_resources_multiple({ | ||
signer: account, | ||
calls: cleanedCalls, |
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.
Consider adding user feedback for transfer errors rather than just logging to console. This could improve user experience by making errors visible.
Failed to generate code suggestions for PR |
Summary by CodeRabbit
New Features
RealmTransfer
component.Improvements
EntityResourceTable
component.Bug Fixes
RealmTransfer
component.