FE-Release 2024-09-26 2 #3200
FE-Release 2024-09-26 2 #3200
Conversation
* rearrange refund validation checks [SLT-185] * test: fix own incorrectly defined tests --------- Co-authored-by: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com>
…SDK gasAirdropAmount [SLT-269] (#3196) * Generalizes airdrop decimal display based on SDK gasAirdropAmount
abtestingalpha
requested review from
bigboydiamonds,
aureliusbtc and
ChiTimesChi
as code owners
WalkthroughThe pull request introduces several updates across multiple packages, primarily focusing on version bumps and enhancements to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning Review ran into problems🔥 ProblemsError running LanguageTool: LanguageTool error: Unknown error 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: 1
🧹 Outside diff range and nitpick comments (4)
packages/synapse-interface/utils/getSignificantDecimals.ts (2)
1-6: Function signature looks good, but consider adding a safety check.The function signature is well-defined with clear types. However, to improve robustness, consider adding a check for the existence of
parts[1]before assigning it todecimalPart.You could modify line 6 as follows:
- const decimalPart = parts[1] + const decimalPart = parts[1] || ''This ensures
decimalPartis always a string, even if there's no decimal point in the input.
1-23: Consider adding documentation for the function.While the function is generally well-implemented, it lacks documentation. Adding a brief comment explaining the function's purpose, parameters, return value, and any important considerations (like how it handles trailing zeros) would improve its maintainability and usability.
Consider adding a JSDoc comment above the function, like this:
/** * Calculates the number of significant decimal places in a given number string. * @param numberString - The number as a string to analyze. * @param defaultDecimals - The number of decimal places to return if the string consists of only zeros after the decimal point. Defaults to 2. * @returns The number of significant decimal places. * @example * getSignificantDecimals("123.4560") // returns 4 * getSignificantDecimals("123.0000", 2) // returns 2 */ export const getSignificantDecimals = ( // ... rest of the functionpackages/synapse-interface/components/StateManagedBridge/BridgeExchangeRateInfo.tsx (1)
210-210: Improved currency-specific decimal handling, but consider scalability.The changes in
getAirdropInDollarsfunction improve the handling of decimal places for different currencies:
- The introduction of the
decimalsvariable allows for currency-specific precision.- Using
decimalsintoFixedensures appropriate rounding for each currency.While this is an improvement, the current implementation might not scale well if more currencies require different decimal precisions.
Consider a more scalable approach, such as:
- Creating a mapping of currencies to their required decimal places.
- Using a default value for currencies not in the mapping.
Example:
const CURRENCY_DECIMALS = { 'JEWEL': 4, // Add more currencies as needed }; const DEFAULT_DECIMALS = 2; // In the function: const decimals = CURRENCY_DECIMALS[symbol] || DEFAULT_DECIMALS;This approach would be more maintainable as new currencies are added.
Also applies to: 216-216
packages/contracts-rfq/test/FastBridgeV2.Src.t.sol (1)
914-914: LGTM with a minor suggestion: Fix typo in function name.The refund call in the
test_refund_revert_justBeforeDeadline_permisionlessfunction correctly tests the expected revert behavior for a permissionless refund attempt just before the deadline.However, there's a typo in the function name:
- function test_refund_revert_justBeforeDeadline_permisionless(address caller) public { + function test_refund_revert_justBeforeDeadline_permissionless(address caller) public {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
- packages/contracts-rfq/CHANGELOG.md (1 hunks)
- packages/contracts-rfq/contracts/FastBridgeV2.sol (2 hunks)
- packages/contracts-rfq/package.json (1 hunks)
- packages/contracts-rfq/test/FastBridgeV2.Src.t.sol (1 hunks)
- packages/synapse-interface/CHANGELOG.md (1 hunks)
- packages/synapse-interface/components/StateManagedBridge/BridgeExchangeRateInfo.tsx (3 hunks)
- packages/synapse-interface/package.json (1 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 (4)
- packages/contracts-rfq/CHANGELOG.md
- packages/contracts-rfq/package.json
- packages/synapse-interface/package.json
- packages/synapse-interface/utils/hooks/useCoingeckoPrice.ts
🔇 Additional comments (11)
packages/synapse-interface/utils/getSignificantDecimals.ts (2)
8-12: Initial checks are well-implemented.The checks for no decimal part and for decimal parts consisting only of zeros are correct and efficient. Using
defaultDecimalsfor strings of all zeros is a good approach, allowing for customizable precision in such cases.
22-22: Return statement is correct.The function correctly returns the count of significant decimals.
packages/synapse-interface/components/StateManagedBridge/BridgeExchangeRateInfo.tsx (3)
14-14: LGTM: New utility function import added.The addition of the
getSignificantDecimalsimport is appropriate for the changes made in theGasDropLabelcomponent. This promotes better code organization by separating utility functions.
146-147: Improved flexibility in gas drop amount display.The changes in the
GasDropLabelcomponent enhance the display of gas drop amounts:
- The introduction of
stringifiedGasAmountand the use ofgetSignificantDecimalsallow for dynamic calculation of decimal places.- Using
significantDecimalsinformatBigIntToStringensures that the displayed amount is always meaningful and appropriate for the value.This approach is more flexible and maintainable than the previous hardcoded logic.
Also applies to: 152-152
Line range hint
1-218: Overall improvements in value display and code flexibility.The changes in this file enhance the display of gas drop amounts and airdrop values:
- The introduction of
getSignificantDecimalsimproves the flexibility of decimal place display for gas drop amounts.- The modifications in
getAirdropInDollarsallow for currency-specific decimal precision.These changes make the code more maintainable and adaptable to different scenarios. The overall structure of the file remains intact, and the changes align well with the provided AI-generated summary.
While the improvements are significant, consider the scalability suggestion for currency-specific decimal handling to future-proof the code.
packages/contracts-rfq/contracts/FastBridgeV2.sol (3)
277-279: Improved security with additional status checkThe new check
if (bridgeStatuses[transactionId] != BridgeStatus.REQUESTED) revert StatusIncorrect();enhances the security of therefundfunction. It ensures that only transactions in theREQUESTEDstate can be refunded, preventing potential issues with transactions in other states.
290-290: Enhanced clarity in status update commentThe updated comment "if all checks passed, set to REFUNDED status" provides better clarity on when the transaction status is set to
REFUNDED. This change accurately reflects the function's logic and improves code readability.
Line range hint
277-290: Summary: Improved refund function security and clarityThe changes to the
refundfunction in theFastBridgeV2contract enhance both security and clarity:
- The new status check ensures that only transactions in the
REQUESTEDstate can be refunded, preventing potential issues with transactions in other states.- The updated comment provides better clarity on when the transaction status is set to
REFUNDED.These improvements align well with the contract's overall design and contribute to its robustness and maintainability.
packages/contracts-rfq/test/FastBridgeV2.Src.t.sol (2)
899-899: LGTM: Refund test implementation looks correct.The refund call in the
test_refund_revert_zeroDelayfunction is correctly implemented, testing the expected revert behavior when attempting a refund with zero delay.
906-906: LGTM: Refund test for just before deadline is properly implemented.The refund call in the
test_refund_revert_justBeforeDeadlinefunction correctly tests the expected revert behavior when attempting a refund just before the deadline.packages/synapse-interface/CHANGELOG.md (1)
6-11: LGTM! Good practices observed in this changelog entry.The changelog entry for version 0.40.1 is well-structured and informative. It's great to see:
- A clear description of the bug fix.
- Reference to the specific ticket (SLT-269) for traceability.
- Link to the exact commit (aa37b50) that implements this change.
These practices make it easier for developers and stakeholders to understand and track changes in the project.
Bundle ReportChanges will decrease total bundle size by 1.76kB (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## fe-release #3200 +/- ##
=====================================================
+ Coverage 38.35432% 90.62247% +52.26815%
=====================================================
Files 424 60 -364
Lines 24464 1237 -23227
Branches 148 148
=====================================================
- Hits 9383 1121 -8262
+ Misses 14335 112 -14223
+ Partials 746 4 -742
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements
Version Updates
@synapsecns/contracts-rfq(0.5.6 to 0.5.7) and@synapsecns/synapse-interface(0.40.0 to 0.40.1).911813d: synapse-interface preview link