feat: adds unwrap daily limit check#96
Conversation
Summary of ChangesHello @martinserts, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a critical enhancement to the unwrapping functionality of the bridge, ensuring that users cannot initiate transactions that would exceed the predefined daily unwrapping limits. By integrating a client-side validation mechanism, the application now proactively fetches the remaining daily limit and validates user input against it. This prevents failed transactions and improves the overall user experience by providing clear, immediate feedback when an attempted unwrap amount is too high, thereby streamlining the unwrapping process and maintaining bridge stability. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a daily unwrapping limit feature by updating the @tari-project/wxtm-bridge-backend-api dependency and integrating its TokensUnwrappedService. The app/page.tsx component now fetches the remaining daily limit, converts it from micro-XTM to XTM, and passes it to the BridgeForm and BridgeInput components. The BridgeInput component includes a new validation rule to prevent users from exceeding this limit, and the useBridgeToTari hook also performs a pre-check against the daily limit before initiating a transaction. Review comments suggest improving error handling in useBridgeToTari by throwing a specific error instead of returning false when the daily limit is exceeded, and refactoring duplicated micro-XTM to XTM conversion logic and magic numbers into shared utility functions and constants for better maintainability and readability.
| if (amountNum > limitXtm) { | ||
| console.error(`[ TAPPLET-BRIDGE ] Daily limit exceeded. Limit: ${limitXtm}, Amount: ${amountNum}`) | ||
| return false | ||
| } |
There was a problem hiding this comment.
Instead of returning false when the daily limit is exceeded, it would be better to throw an error with a specific message. This would allow the UI to display a more informative error to the user, rather than a generic failure message. This would also make it consistent with the error handling in handleBridgeToEthereum.
Then, you'll need to update the catch block at the end of the bridgeToTari function to re-throw the error, and update handleBridgeToTari in app/page.tsx to handle rejected promises, similar to how handleBridgeToEthereum does.
if (amountNum > limitXtm) {
throw new Error(`[ TAPPLET-BRIDGE ] Daily limit exceeded. Limit: ${limitXtm}, Amount: ${amountNum}`);
}There was a problem hiding this comment.
Added the change to throw error - 6183f89
But I will not rethrow, since callers of bridgeToTari() do not expect exceptions.
| const fetchDailyLimit = async () => { | ||
| try { | ||
| const limitMicro = await TokensUnwrappedService.getRemainingDailyLimit() | ||
| const limitXtm = Number(BigInt(limitMicro) / BigInt(1_000_000)) | ||
| setRemainingDailyLimit(limitXtm) | ||
| } catch (error) { | ||
| console.error('[ TAPPLET-BRIDGE ] Failed to fetch daily limit:', error) | ||
| } | ||
| } |
There was a problem hiding this comment.
This logic for converting microXTM to XTM, and the magic number 1_000_000, are duplicated in hooks/use-bridge-to-tari.ts. To improve maintainability, this should be refactored.
Consider the following:
- Define a constant for
1_000_000in a shared location likeconfig/index.ts. For example:export const MICRO_XTM_PER_XTM = 1_000_000; - Create a utility function for the conversion in a shared file (e.g.,
utils/parse-wxtm-token-amount.ts):import { MICRO_XTM_PER_XTM } from '@/config'; export const microXtmToXtm = (microXtm: string | bigint): number => { return Number(BigInt(microXtm) / BigInt(MICRO_XTM_PER_XTM)); };
- Use this new utility function here and in
hooks/use-bridge-to-tari.ts.
Additionally, the interval 30000 on line 170 could also be extracted into a named constant (e.g., REFETCH_LIMIT_INTERVAL) to improve readability.
| const limitMicro = await TokensUnwrappedService.getRemainingDailyLimit() | ||
| const limitXtm = Number(BigInt(limitMicro) / BigInt(1_000_000)) |
There was a problem hiding this comment.
This logic for fetching and converting the daily limit is duplicated from app/page.tsx. As suggested in the comment on that file, this should be refactored into a shared utility function to avoid code duplication and magic numbers.
2 participants
Description
Fetches remaining daily limit for unwrapping.
Will reject amounts, which exceed daily limit with an error message.
Motivation and Context
Users could Unwrap any amount as long as they had enough wXTM in their wallet.
Our Bridge has daily limits. So if anyone tried to bridge amounts, which exceed daily limit, bridge just got stuck.
This change limits the amounts users can bridge.
How Has This Been Tested?
Decreased daily limit to 100 in admin UI.
Tried to unwrap an amount, which exceeds it.
Breaking Changes