fix(synapse-interface): Generalizes airdrop decimal display based on SDK gasAirdropAmount [SLT-269] #3196
Conversation
abtestingalpha
requested review from
bigboydiamonds and
aureliusbtc
as code owners
WalkthroughThe changes involve updates to the Changes
Possibly related PRs
Suggested labels
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
|
![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 (4)
packages/synapse-interface/utils/getSignificantDecimals.ts (2)
8-12: LGTM: Edge cases are handled correctly.The function properly handles cases where there's no decimal part or the decimal part consists only of zeros. The use of a regular expression for checking all zeros is efficient.
Consider combining the two checks for a slight optimization:
- if (!decimalPart) return 0 - - if (/^0*$/.test(decimalPart)) { - return defaultDecimals - } + if (!decimalPart || /^0*$/.test(decimalPart)) { + return decimalPart ? defaultDecimals : 0 + }This change reduces the number of early returns and combines related logic.
14-20: LGTM: Main logic for counting significant decimals is correct.The function correctly iterates through the decimal part, updating
significantDecimalswhen a non-zero digit is found. This approach accurately identifies the last non-zero digit, which determines the number of significant decimal places.Consider using a reverse iteration for a potential optimization:
- let significantDecimals = 0 - - for (let i = 0; i < decimalPart.length; i++) { - if (decimalPart[i] !== '0') { - significantDecimals = i + 1 - } - } + for (let i = decimalPart.length - 1; i >= 0; i--) { + if (decimalPart[i] !== '0') { + return i + 1 + } + } + return 0This change allows the function to return as soon as it finds the last non-zero digit, potentially improving performance for large decimal parts.
packages/synapse-interface/components/StateManagedBridge/BridgeExchangeRateInfo.tsx (2)
14-14: Improved flexibility in gas drop amount displayThe changes to dynamically calculate significant decimals for the gas drop amount display are a good improvement. This approach is more adaptable to different gas drop amounts and chains, aligning well with the PR objective.
Consider extracting the gas drop amount formatting logic into a separate utility function for better reusability and testability. For example:
const formatGasDropAmount = (amount: bigint): string => { const stringifiedAmount = formatBigIntToString(amount, 18); const significantDecimals = getSignificantDecimals(stringifiedAmount); return formatBigIntToString(amount, 18, significantDecimals); };Then you can use it in the component like this:
const formattedGasDropAmount = formatGasDropAmount(gasDropAmount);This would make the component cleaner and the formatting logic easier to test and maintain.
Also applies to: 146-147, 152-152
210-210: Improved precision for airdrop value displayThe changes to dynamically set the decimal precision based on the symbol are a good improvement. This allows for more accurate representation of the airdrop value, especially for the 'JEWEL' symbol.
While this change is good, consider future-proofing this approach:
- Instead of hardcoding the decimal places, consider creating a configuration object or function that maps symbols to their required decimal places. This would make it easier to add or modify precision for different symbols in the future.
- You might want to add a default case for unknown symbols. For example:
const getSymbolDecimals = (symbol: string): number => { const decimalMap: Record<string, number> = { JEWEL: 4, // Add other symbols here as needed }; return decimalMap[symbol] || 2; // Default to 2 decimals if symbol is not found }; const decimals = getSymbolDecimals(symbol);This approach would make the code more maintainable and extensible for future symbol additions or changes.
Also applies to: 216-216
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- packages/synapse-interface/components/StateManagedBridge/BridgeExchangeRateInfo.tsx (3 hunks)
- packages/synapse-interface/utils/getSignificantDecimals.ts (1 hunks)
- packages/synapse-interface/utils/hooks/useCoingeckoPrice.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/synapse-interface/utils/hooks/useCoingeckoPrice.ts
🔇 Additional comments (3)
packages/synapse-interface/utils/getSignificantDecimals.ts (3)
1-7: LGTM: Function signature and initial parsing are well-defined.The function signature is clear, uses proper TypeScript types, and provides a sensible default for
defaultDecimals. The initial parsing of the input string is correct for isolating the decimal part.
22-23: LGTM: Return statement is correct.The function properly returns the calculated
significantDecimals.
1-23: Overall, the implementation is solid and aligns with the PR objectives.The
getSignificantDecimalsfunction provides a flexible way to determine significant decimals, which can effectively improve the display of gas airdrop amounts. It handles various edge cases and should work correctly for different input formats. The implementation is clear and maintainable.To ensure the function works as expected, consider adding the following test cases:
console.log(getSignificantDecimals('1.00000')); // Expected: 0 console.log(getSignificantDecimals('1.10000')); // Expected: 1 console.log(getSignificantDecimals('1.10001')); // Expected: 5 console.log(getSignificantDecimals('1')); // Expected: 0 console.log(getSignificantDecimals('1.000', 4)); // Expected: 4These test cases cover various scenarios including trailing zeros, no decimal part, and custom default decimals.
Deploying sanguine-fe with
|
| Latest commit: |
a02f800
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8f3ea9f2.sanguine-fe.pages.dev |
| Branch Preview URL: | https://fe-airdrop-decimals.sanguine-fe.pages.dev |
Deploying sanguine with
|
| Latest commit: |
d96f06d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://426097fc.sanguine.pages.dev |
| Branch Preview URL: | https://fe-airdrop-decimals.sanguine.pages.dev |
Bundle ReportChanges will increase total bundle size by 155.06kB (0.43%) ⬆️. This is within the configured threshold ✅ Detailed changes
ℹ️ *Bundle size includes cached data from a previous commit |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3196 +/- ##
====================================================
+ Coverage 40.97804% 90.56974% +49.59169%
====================================================
Files 459 54 -405
Lines 25643 1018 -24625
Branches 343 82 -261
====================================================
- Hits 10508 922 -9586
+ Misses 14383 93 -14290
+ Partials 752 3 -749
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Just to confirm, we are returning the last non-zero decimal number digit here?
There was a problem hiding this comment.
The function counts how many digits are important, starting from the left and then stops at the last important non-zero number. So, it returns how many important digits there are.
Couple examples
- For
0.002000000000000000:
- The decimal part is
002000000000000000 - The last non-zero digit is 2 (in 3rd position)
- So function returns 3
- For
1.000000000000000000:
- Decimal part is
000000000000000000 - Since all decimal digits are zero it returns
defaultDecimals(2). This is for Canto, so the airdrop shows1.00instead of just1.
Description
Generalizes the gas airdrop shown based on size and format of amount.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
429cd63: synapse-interface preview link
9937a7a: synapse-interface preview link
357e2f6: synapse-interface preview link
a0a1d5a: synapse-interface preview link
d8e9d7b: synapse-interface preview link