[TASK-6919] Show slippage fee on request pay#526
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request includes updates to the Changes
Possibly related PRs
Suggested reviewers
Warning Rate limit exceeded@jjramirezn has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 59 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 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 (
|
Hugo0
left a comment
Hugo0
left a comment
There was a problem hiding this comment.
lgtm, dependent on SDK release
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
package.json (1)
26-26: Consider SDK update timingThe PR objectives indicate that the SDK version should be updated following the deployment to the main branch. Consider whether this update should be in a separate PR that's merged after the main deployment.
src/components/Request/Pay/Views/Initial.view.tsx (2)
143-153: Remove console logging statementsProduction code should not contain console logging statements. Consider using a proper logging system if debugging information needs to be preserved.
- console.log('slippage', slippagePercentage) - console.dir(txData)
424-439: Consider adding loading state for slippage calculationThe slippage cost display is well-implemented and consistent with the existing UI. However, consider adding a loading state similar to the network cost display to improve user experience during calculation.
- {null !== calculatedSlippage && ( + {calculatedSlippage === null ? ( + <div className="flex w-full flex-row items-center justify-between gap-1 px-2 text-h8 text-gray-1"> + <div className="flex w-max flex-row items-center justify-center gap-1"> + <Icon name={'money-out'} className="h-4 fill-gray-1" /> + <label className="font-bold">Slippage cost</label> + </div> + <div className="h-2 w-16 animate-colorPulse rounded bg-slate-700"></div> + </div> + ) : calculatedSlippage && (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
package.json(1 hunks)src/components/Request/Pay/Views/Initial.view.tsx(5 hunks)
🔇 Additional comments (5)
package.json (1)
26-26:
Update SDK version to 0.5.10 and include lock file changes
The SDK version needs to be updated to ^0.5.10 due to a recent hotfix. Additionally, the pnpm-lock.json file must be updated to reflect this change.
Apply this diff:
- "@squirrel-labs/peanut-sdk": "^0.5.9",
+ "@squirrel-labs/peanut-sdk": "^0.5.10",Then run:
pnpm installto update the lock file.
src/components/Request/Pay/Views/Initial.view.tsx (4)
93-96: LGTM: Well-structured state management for slippage handling
The new state variables are properly typed and initialized, following React's best practices.
102-106: LGTM: Efficient slippage calculation with proper null handling
The calculation is correctly memoized and handles undefined states appropriately. The result is properly formatted for currency display.
187-188: LGTM: Proper state cleanup on token/chain changes
The slippage states are appropriately reset when the user changes the token or chain, preventing stale data from being displayed.
308-316: LGTM: Safe transaction handling with proper checks
The code properly checks for the existence of cross-chain transactions before proceeding and correctly integrates with the transaction submission flow.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/components/Request/Pay/Views/Initial.view.tsx (2)
102-106: Consider using a more precise numeric handling approachWhile the current implementation is functional, consider using a decimal handling library like
decimal.jsorbignumber.jsfor more precise financial calculations to avoid potential floating-point precision issues.const calculatedSlippage = useMemo(() => { if (!selectedTokenData?.price || !slippagePercentage) return null - return ((slippagePercentage / 100) * selectedTokenData.price * Number(estimatedFromValue)).toFixed(2) + return new BigNumber(slippagePercentage) + .div(100) + .times(selectedTokenData.price) + .times(estimatedFromValue) + .toFixed(2) }, [slippagePercentage, selectedTokenData, estimatedFromValue])
306-314: Consider enhancing error feedback for cross-chain transactionsWhile the implementation is correct, consider providing more specific error messages when
xChainUnsignedTxsis undefined to improve the user experience.- if (!xChainUnsignedTxs) return + if (!xChainUnsignedTxs) { + throw new Error('Cross-chain transaction data is not available. Please try again.') + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.json(1 hunks)src/components/Request/Pay/Views/Initial.view.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🔇 Additional comments (4)
src/components/Request/Pay/Views/Initial.view.tsx (4)
93-96: LGTM: Well-structured state management for slippage handling
The new state variables are properly typed and initialized, following React's best practices.
143-151: LGTM: Robust transaction preparation with proper error handling
The transaction preparation logic properly handles both success and error cases, ensuring state consistency by resetting values when needed.
Also applies to: 158-159
185-186: LGTM: Clean state reset on token/chain changes
Appropriate reset of slippage-related states when the token or chain selection changes, preventing stale data issues.
422-437: LGTM: Well-implemented slippage cost display
The UI implementation for slippage cost follows the existing design patterns and provides clear information to users through tooltips.
2 participants
Get the slippage fee used from the sdk and show it cost in dollars on request pay
The sdk version must be bumped after deploying to main peanutprotocol/peanut-sdk#166